Fix incorrect evaluation of resolution media queries

Source/core/css/MediaQueryEvaluator.cpp

Index: Source/core/css/MediaQueryEvaluator.cpp
diff --git a/Source/core/css/MediaQueryEvaluator.cpp b/Source/core/css/MediaQueryEvaluator.cpp
index 21574b66c5040fe89ad21f753afc42e1e1ec5894..e052390e23683515689f0b3f4900541938af43f3 100644
--- a/Source/core/css/MediaQueryEvaluator.cpp
+++ b/Source/core/css/MediaQueryEvaluator.cpp
@@ -195,23 +195,6 @@ static bool compareAspectRatioValue(CSSValue* value, int width, int height, Medi
     return false;
 }
 
-#if ENABLE(RESOLUTION_MEDIA_QUERY)
-static bool compareResolution(float min, float max, float value, MediaFeaturePrefix op)
-{
-    switch (op) {
-    case NoPrefix:
-        // A 'resolution' (without a "min-" or "max-" prefix) query
-        // never matches a device with non-square pixels.
-        return value == min && value == max;
-    case MinPrefix:
-        return min >= value;
-    case MaxPrefix:
-        return max <= value;
-    }
-    return false;
-}
-#endif
-
 static bool numberValue(CSSValue* value, float& result)
 {
     if (value->isPrimitiveValue()
@@ -285,7 +268,7 @@ static bool device_aspect_ratioMediaFeatureEval(CSSValue* value, RenderStyle*, F
     return true;
 }
 
-static bool device_pixel_ratioMediaFeatureEval(CSSValue *value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
+static bool evalResolution(CSSValue* value, Frame* frame, MediaFeaturePrefix op)
 {
     // FIXME: Possible handle other media types than 'screen' and 'print'.
     float deviceScaleFactor = 0;
@@ -307,108 +290,44 @@ static bool device_pixel_ratioMediaFeatureEval(CSSValue *value, RenderStyle*, Fr
     if (!value)
         return !!deviceScaleFactor;
 
-    return value->isPrimitiveValue() && compareValue(deviceScaleFactor, static_cast<CSSPrimitiveValue*>(value)->getFloatValue(), op);
-}
-
-static bool resolutionMediaFeatureEval(CSSValue* value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
-{
-#if ENABLE(RESOLUTION_MEDIA_QUERY)
-    // The DPI below is dots per CSS inch and thus not device inch. The
-    // functions should respect this.
-    //
-    // For square pixels, it is simply the device scale factor (dppx) times 96,
-    // per definition.
-    //
-    // The device scale factor is a predefined value which is calculated per
-    // device given the preferred distance in arms length (considered one arms
-    // length for desktop computers and usually 0.6 arms length for phones).
-    //
-    // The value can be calculated as follows (rounded to quarters):
-    //     round((deviceDotsPerInch * distanceInArmsLength / 96) * 4) / 4.
-    // Example (mid-range resolution phone):
-    //     round((244 * 0.6 / 96) * 4) / 4 = 1.5
-    // Example (high-range resolution laptop):
-    //     round((220 * 1.0 / 96) * 4) / 4 = 2.0
-
-    float horiDPI;
-    float vertDPI;
-
-    // This checks the actual media type applied to the document, and we know
-    // this method only got called if this media type matches the one defined
-    // in the query. Thus, if if the document's media type is "print", the
-    // media type of the query will either be "print" or "all".
-    String mediaType = frame->view()->mediaType();
-    if (equalIgnoringCase(mediaType, "screen")) {
-        Screen* screen = frame->document()->domWindow()->screen();
-        horiDPI = screen->horizontalDPI();
-        vertDPI = screen->verticalDPI();
-    } else if (equalIgnoringCase(mediaType, "print")) {
-        // The resolution of images while printing should not depend on the dpi
-        // of the screen. Until we support proper ways of querying this info
-        // we use 300px which is considered minimum for current printers.
-        horiDPI = vertDPI = 300;
-    } else {
-        // FIXME: Possible handle other media types than 'screen' and 'print'.
-        // For now, do not match.
+    if (!value->isPrimitiveValue())
         return false;
-    }
 
-    float leastDenseDPI = std::min(horiDPI, vertDPI);
-    float mostDenseDPI = std::max(horiDPI, vertDPI);
-
-    // According to spec, (resolution) will evaluate to true if (resolution:x)
-    // will evaluate to true for a value x other than zero or zero followed by
-    // a valid unit identifier (i.e., other than 0, 0dpi, 0dpcm, or 0dppx.),
-    // which is always the case. But the spec special cases 'resolution' to
-    // never matches a device with non-square pixels.
-    if (!value) {
-        ASSERT(op == NoPrefix);
-        return leastDenseDPI == mostDenseDPI;
-    }
+    CSSPrimitiveValue* resolution = static_cast<CSSPrimitiveValue*>(value);
 
-    if (!value->isPrimitiveValue())
-        return false;
+    if (resolution->isNumber())
+        compareValue(deviceScaleFactor, resolution->getFloatValue(), op);
 
-    // http://dev.w3.org/csswg/css3-values/#resolution defines resolution as a
-    // dimension, which contains a number (decimal point allowed), not just an
-    // integer. Also, http://dev.w3.org/csswg/css3-values/#numeric-types says
-    // "CSS theoretically supports infinite precision and infinite ranges for
-    // all value types;
-    CSSPrimitiveValue* rawValue = static_cast<CSSPrimitiveValue*>(value);
-
-    if (rawValue->isDotsPerPixel()) {
-        // http://dev.w3.org/csswg/css3-values/#absolute-lengths recommends
-        // "that the pixel unit refer to the whole number of device pixels that
-        // best approximates the reference pixel". We compare with 3 decimal
-        // points, which aligns with current device-pixel-ratio's in use.
-        float leastDenseDensity = floorf(leastDenseDPI * 1000 / 96) / 1000;
-        float mostDenseDensity = floorf(leastDenseDPI * 1000 / 96) / 1000;
-        float testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPPX);
-        return compareResolution(leastDenseDensity, mostDenseDensity, testedDensity, op);
+    if (resolution->isResolution()) {
+        // To match DPCM and DPI integer values to DPPX values, we limit to 2 decimal
+        // points. The http://dev.w3.org/csswg/css3-values/#absolute-lengths recommends
+        // "that the pixel unit refer to the whole number of device pixels that best
+        // approximates the reference pixel". With that in mind, allowing 2 decimal
+        // point precision seems appropriate.
+        return compareValue(
+            floorf(0.5 + 100 * deviceScaleFactor) / 100,
+            floorf(0.5 + 100 * resolution->getFloatValue(CSSPrimitiveValue::CSS_DPPX)) / 100, op);

[johnme, 2013/04/23 18:16:23]

I don't understand this line. A resolution for which isResolution is true can be
of type DPCM or DPI in addition to DPPX. So how does
resolution->getFloatValue(CSSPrimitiveValue::CSS_DPPX) work? getFloatValue
doesn't seem to do any unit conversions, unless I'm missing something?

     }
 
-    if (rawValue->isDotsPerInch()) {
-        unsigned testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPI);
-        return compareResolution(leastDenseDPI, mostDenseDPI, testedDensity, op);
-    }
+    return false;
+}
 
-    // http://dev.w3.org/csswg/css3-values/#absolute-lengths recommends "that
-    // the pixel unit refer to the whole number of device pixels that best
-    // approximates the reference pixel".
-    float leastDenseDPCM = roundf(leastDenseDPI / 2.54); // (2.54 cm/in)
-    float mostDenseDPCM = roundf(mostDenseDPI / 2.54);
+static bool device_pixel_ratioMediaFeatureEval(CSSValue *value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
+{
+    return (!value || static_cast<CSSPrimitiveValue*>(value)->isNumber()) && evalResolution(value, frame, op);
+}
 
-    if (rawValue->isDotsPerCentimeter()) {
-        float testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPCM);
-        return compareResolution(leastDenseDPCM, mostDenseDPCM, testedDensity, op);
-    }
+static bool resolutionMediaFeatureEval(CSSValue* value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
+{
+#if ENABLE(RESOLUTION_MEDIA_QUERY)
+    return (!value || static_cast<CSSPrimitiveValue*>(value)->isResolution()) && evalResolution(value, frame, op);
 #else
     UNUSED_PARAM(value);
     UNUSED_PARAM(frame);
     UNUSED_PARAM(op);
-#endif
 
     return false;
+#endif
 }
 
 static bool gridMediaFeatureEval(CSSValue* value, RenderStyle*, Frame*, MediaFeaturePrefix op)