Changeset 276208 in webkit

Timestamp:

Apr 17, 2021, 3:21:20 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Media queries with max-width greater than 999999999px evaluate to false
​https://bugs.webkit.org/show_bug.cgi?id=224097

Patch by Tyler Wilcock <Tyler Wilcock> on 2021-04-17
Reviewed by Darin Adler.

We now evaluate <length> values in media queries with double
Source/WebCore:

precision instead of int precision to match other browsers and pass a WPT.

See similar method in Chromium:
​https://github.com/chromium/chromium/blob/09a0b960b27f6e08fbe67ad97e6c4fb55ada383f/third_party/blink/renderer/core/css/media_query_evaluator.cc#L436

Test: fast/media/media-query-lengths-evaluate-with-double-precision.html
and WPT imported/w3c/web-platform-tests/css/mediaqueries/min-width-001.xht

• css/MediaQueryEvaluator.cpp:

(WebCore::computeLength):
Return Optional<double> rather than int& out-value.

(WebCore::deviceHeightEvaluate):
(WebCore::deviceWidthEvaluate):
(WebCore::heightEvaluate):
(WebCore::widthEvaluate):
Evaluate length values as doubles instead of ints.

LayoutTests:

precision instead of int precision to match other browsers and pass another WPT.

• TestExpectations:

Remove ImageOnlyFailure for imported/w3c/web-platform-tests/css/mediaqueries/min-width-001.xht
because it passes now.

• fast/media/media-query-lengths-evaluate-with-double-precision-expected.html:

Added.

• fast/media/media-query-lengths-evaluate-with-double-precision.html:

Added to test properties that WPT min-width-001.xht doesn't
(min-height, device-min-height, device-min-width).

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision-expected.html (added)
• LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/MediaQueryEvaluator.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-04-17  Tyler Wilcock  <twilco.o@protonmail.com>
+
+        Media queries with max-width greater than 999999999px evaluate to false
+        https://bugs.webkit.org/show_bug.cgi?id=224097
+
+        Reviewed by Darin Adler.
+
+        We now evaluate <length> values in media queries with double
+        precision instead of int precision to match other browsers and pass another WPT.
+
+        * TestExpectations:
+        Remove ImageOnlyFailure for imported/w3c/web-platform-tests/css/mediaqueries/min-width-001.xht
+        because it passes now.
+
+        * fast/media/media-query-lengths-evaluate-with-double-precision-expected.html:
+        Added.
+
+        * fast/media/media-query-lengths-evaluate-with-double-precision.html:
+        Added to test properties that WPT min-width-001.xht doesn't
+        (min-height, device-min-height, device-min-width).
+
 2021-04-17  Philippe Normand  <pnormand@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -1253 +1253 @@
 imported/w3c/web-platform-tests/css/mediaqueries/device-aspect-ratio-005.html [ Skip ]
 
-imported/w3c/web-platform-tests/css/mediaqueries/min-width-001.xht [ ImageOnlyFailure ]
 webkit.org/b/156684 imported/w3c/web-platform-tests/css/mediaqueries/relative-units-001.html [ ImageOnlyFailure ]
 webkit.org/b/156684 imported/w3c/web-platform-tests/css/mediaqueries/mq-calc-005.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-04-17  Tyler Wilcock  <twilco.o@protonmail.com>
+
+        Media queries with max-width greater than 999999999px evaluate to false
+        https://bugs.webkit.org/show_bug.cgi?id=224097
+
+        Reviewed by Darin Adler.
+
+        We now evaluate <length> values in media queries with double
+        precision instead of int precision to match other browsers and pass a WPT.
+
+        See similar method in Chromium:
+        https://github.com/chromium/chromium/blob/09a0b960b27f6e08fbe67ad97e6c4fb55ada383f/third_party/blink/renderer/core/css/media_query_evaluator.cc#L436
+
+        Test: fast/media/media-query-lengths-evaluate-with-double-precision.html
+        and WPT imported/w3c/web-platform-tests/css/mediaqueries/min-width-001.xht
+
+        * css/MediaQueryEvaluator.cpp:
+        (WebCore::computeLength):
+        Return Optional<double> rather than int& out-value.
+
+        (WebCore::deviceHeightEvaluate):
+        (WebCore::deviceWidthEvaluate):
+        (WebCore::heightEvaluate):
+        (WebCore::widthEvaluate):
+        Evaluate `length` values as doubles instead of ints.
+
 2021-04-17  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/css/MediaQueryEvaluator.cpp

@@ -483 +483 @@
 }
 
-static bool computeLength(CSSValue* value, bool strict, const CSSToLengthConversionData& conversionData, int& result)
+static Optional<double> computeLength(CSSValue* value, bool strict, const CSSToLengthConversionData& conversionData)
 {
     if (!is<CSSPrimitiveValue>(value))
-        return false;
+        return WTF::nullopt;
 
     auto& primitiveValue = downcast<CSSPrimitiveValue>(*value);
-
     if (primitiveValue.isNumber()) {
-        result = primitiveValue.intValue();
-        return !strict || !result;
-    }
-
-    if (primitiveValue.isLength()) {
-        result = primitiveValue.computeLength<int>(conversionData);
-        return true;
-    }
-
-    return false;
+        double value = primitiveValue.doubleValue();
+        // The only unitless number value allowed in strict mode is zero.
+        if (strict && value)
+            return WTF::nullopt;
+        return value;
+    }
+
+    if (primitiveValue.isLength())
+        return primitiveValue.computeLength<double>(conversionData);
+
+    return WTF::nullopt;
 }
 
@@ -509 +509 @@
     if (!value)
         return true;
-    int length;
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
+        return false;
+
     auto height = frame.mainFrame().screenSize().height();
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
-        return false;
-
-    LOG_WITH_STREAM(MediaQueries, stream << "  deviceHeightEvaluate: query " << op << " height " << length << ", actual height " << height << " result: " << compareValue(height, length, op));
-
-    return compareValue(height, length, op);
+    LOG_WITH_STREAM(MediaQueries, stream << "  deviceHeightEvaluate: query " << op << " height " << *length << ", actual height " << height << " result: " << compareValue(height, *length, op));
+    return compareValue(height, *length, op);
 }
 
@@ -525 +524 @@
     if (!value)
         return true;
-    int length;
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
+        return false;
+
     auto width = frame.mainFrame().screenSize().width();
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
-        return false;
-
-    LOG_WITH_STREAM(MediaQueries, stream << "  deviceWidthEvaluate: query " << op << " width " << length << ", actual width " << width << " result: " << compareValue(width, length, op));
-
-    return compareValue(width, length, op);
+    LOG_WITH_STREAM(MediaQueries, stream << "  deviceWidthEvaluate: query " << op << " width " << *length << ", actual width " << width << " result: " << compareValue(width, *length, op));
+    return compareValue(width, *length, op);
 }
 
@@ -543 +541 @@
     if (!value)
         return height;
+
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
+        return false;
+
     if (auto* renderView = frame.document()->renderView())
         height = adjustForAbsoluteZoom(height, *renderView);
 
-    int length;
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
-        return false;
-
-    LOG_WITH_STREAM(MediaQueries, stream << "  heightEvaluate: query " << op << " height " << length << ", actual height " << height << " result: " << compareValue(height, length, op));
-
-    return compareValue(height, length, op);
+    LOG_WITH_STREAM(MediaQueries, stream << "  heightEvaluate: query " << op << " height " << *length << ", actual height " << height << " result: " << compareValue(height, *length, op));
+    return compareValue(height, *length, op);
 }
 
@@ -563 +561 @@
     if (!value)
         return width;
+
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
+        return false;
+
     if (auto* renderView = frame.document()->renderView())
         width = adjustForAbsoluteZoom(width, *renderView);
 
-    int length;
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
-        return false;
-
-    LOG_WITH_STREAM(MediaQueries, stream << "  widthEvaluate: query " << op << " width " << length << ", actual width " << width << " result: " << compareValue(width, length, op));
-
-    return compareValue(width, length, op);
+    LOG_WITH_STREAM(MediaQueries, stream << "  widthEvaluate: query " << op << " width " << *length << ", actual width " << width << " result: " << compareValue(width, *length, op));
+    return compareValue(width, *length, op);
 }