Title: [276208] trunk
Revision
276208
Author
[email protected]
Date
2021-04-17 15:21:20 -0700 (Sat, 17 Apr 2021)

Log 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 <[email protected]> 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).

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (276207 => 276208)


--- trunk/LayoutTests/ChangeLog	2021-04-17 21:13:20 UTC (rev 276207)
+++ trunk/LayoutTests/ChangeLog	2021-04-17 22:21:20 UTC (rev 276208)
@@ -1,3 +1,24 @@
+2021-04-17  Tyler Wilcock  <[email protected]>
+
+        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  <[email protected]>
 
         [GStreamer][MediaStream] fast/mediastream/play-newly-added-audio-track.html is failing since added in r260380

Modified: trunk/LayoutTests/TestExpectations (276207 => 276208)


--- trunk/LayoutTests/TestExpectations	2021-04-17 21:13:20 UTC (rev 276207)
+++ trunk/LayoutTests/TestExpectations	2021-04-17 22:21:20 UTC (rev 276208)
@@ -1252,7 +1252,6 @@
 imported/w3c/web-platform-tests/css/mediaqueries/device-aspect-ratio-001.html [ Skip ]
 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 ]
 

Added: trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision-expected.html (0 => 276208)


--- trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision-expected.html	2021-04-17 22:21:20 UTC (rev 276208)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Test: @media <length> values are evaluated with greater-than-int precision.</title>
+    <meta name="assert" content="All boxes are blue.">
+    <style>
+        div {
+            display: inline-block;
+            width: 250px;
+            height: 250px;
+            margin: 30px;
+            background-color: blue;
+        }
+    </style>
+</head>
+<body>
+<div></div>
+<div></div>
+<div></div>
+<div></div>
+</body>
+</html>
+

Added: trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision.html (0 => 276208)


--- trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision.html	                        (rev 0)
+++ trunk/LayoutTests/fast/media/media-query-lengths-evaluate-with-double-precision.html	2021-04-17 22:21:20 UTC (rev 276208)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Test: @media <length> values are evaluated with greater-than-int precision.</title>
+    <meta name="assert" content="All boxes are blue.">
+    <style>
+        div {
+            display: inline-block;
+            width: 250px;
+            height: 250px;
+            margin: 30px;
+            background-color: blue;
+        }
+        @media (min-width: 99999999999px) {
+            #box-one {
+                background-color: yellow;
+            }
+        }
+        @media (min-height: 99999999999px) {
+            #box-two {
+                background-color: yellow;
+            }
+        }
+        @media (min-device-width: 99999999999px) {
+            #box-three {
+                background-color: yellow;
+            }
+        }
+        @media (min-device-height: 99999999999px) {
+            #box-four {
+                background-color: yellow;
+            }
+        }
+    </style>
+</head>
+<body>
+<div id="box-one"></div>
+<div id="box-two"></div>
+<div id="box-three"></div>
+<div id="box-four"></div>
+</body>
+</html>
+

Modified: trunk/Source/WebCore/ChangeLog (276207 => 276208)


--- trunk/Source/WebCore/ChangeLog	2021-04-17 21:13:20 UTC (rev 276207)
+++ trunk/Source/WebCore/ChangeLog	2021-04-17 22:21:20 UTC (rev 276208)
@@ -1,3 +1,29 @@
+2021-04-17  Tyler Wilcock  <[email protected]>
+
+        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  <[email protected]>
 
         [Cleanup] Remove redundant BreakingContext::m_currentStyle

Modified: trunk/Source/WebCore/css/MediaQueryEvaluator.cpp (276207 => 276208)


--- trunk/Source/WebCore/css/MediaQueryEvaluator.cpp	2021-04-17 21:13:20 UTC (rev 276207)
+++ trunk/Source/WebCore/css/MediaQueryEvaluator.cpp	2021-04-17 22:21:20 UTC (rev 276208)
@@ -482,24 +482,24 @@
     return zeroEvaluate(value, op);
 }
 
-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;
+        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()) {
-        result = primitiveValue.computeLength<int>(conversionData);
-        return true;
-    }
+    if (primitiveValue.isLength())
+        return primitiveValue.computeLength<double>(conversionData);
 
-    return false;
+    return WTF::nullopt;
 }
 
 static bool deviceHeightEvaluate(CSSValue* value, const CSSToLengthConversionData& conversionData, Frame& frame, MediaFeaturePrefix op)
@@ -508,14 +508,13 @@
     // assume if we have a device, assume non-zero
     if (!value)
         return true;
-    int length;
-    auto height = frame.mainFrame().screenSize().height();
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!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);
+    auto height = frame.mainFrame().screenSize().height();
+    LOG_WITH_STREAM(MediaQueries, stream << "  deviceHeightEvaluate: query " << op << " height " << *length << ", actual height " << height << " result: " << compareValue(height, *length, op));
+    return compareValue(height, *length, op);
 }
 
 static bool deviceWidthEvaluate(CSSValue* value, const CSSToLengthConversionData& conversionData, Frame& frame, MediaFeaturePrefix op)
@@ -524,14 +523,13 @@
     // assume if we have a device, assume non-zero
     if (!value)
         return true;
-    int length;
-    auto width = frame.mainFrame().screenSize().width();
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!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);
+    auto width = frame.mainFrame().screenSize().width();
+    LOG_WITH_STREAM(MediaQueries, stream << "  deviceWidthEvaluate: query " << op << " width " << *length << ", actual width " << width << " result: " << compareValue(width, *length, op));
+    return compareValue(width, *length, op);
 }
 
 static bool heightEvaluate(CSSValue* value, const CSSToLengthConversionData& conversionData, Frame& frame, MediaFeaturePrefix op)
@@ -542,16 +540,16 @@
     int height = view->layoutHeight();
     if (!value)
         return height;
-    if (auto* renderView = frame.document()->renderView())
-        height = adjustForAbsoluteZoom(height, *renderView);
 
-    int length;
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
         return false;
 
-    LOG_WITH_STREAM(MediaQueries, stream << "  heightEvaluate: query " << op << " height " << length << ", actual height " << height << " result: " << compareValue(height, length, op));
+    if (auto* renderView = frame.document()->renderView())
+        height = adjustForAbsoluteZoom(height, *renderView);
 
-    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);
 }
 
 static bool widthEvaluate(CSSValue* value, const CSSToLengthConversionData& conversionData, Frame& frame, MediaFeaturePrefix op)
@@ -562,16 +560,16 @@
     int width = view->layoutWidth();
     if (!value)
         return width;
-    if (auto* renderView = frame.document()->renderView())
-        width = adjustForAbsoluteZoom(width, *renderView);
 
-    int length;
-    if (!computeLength(value, !frame.document()->inQuirksMode(), conversionData, length))
+    auto length = computeLength(value, !frame.document()->inQuirksMode(), conversionData);
+    if (!length)
         return false;
 
-    LOG_WITH_STREAM(MediaQueries, stream << "  widthEvaluate: query " << op << " width " << length << ", actual width " << width << " result: " << compareValue(width, length, op));
+    if (auto* renderView = frame.document()->renderView())
+        width = adjustForAbsoluteZoom(width, *renderView);
 
-    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);
 }
 
 static bool minColorEvaluate(CSSValue* value, const CSSToLengthConversionData& conversionData, Frame& frame, MediaFeaturePrefix)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to