Filter functions grayscale/invert/opacity/sepia should clamp values over 100%, not...
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Nov 2016 18:26:26 +0000 (18:26 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Nov 2016 18:26:26 +0000 (18:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164310
Source/WebCore:

Reviewed by Sam Weinig.

When bringing up the new CSS parser, I discovered that our old parser was
not conforming to the specification.

Covered by existing tests.

* css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseBuiltinFilterArguments): For these functions, clamp to
100% rather than fail.

LayoutTests:

<rdar://problems/29057705>

Reviewed by Sam Weinig.

Some of our tests were incorrectly suggesting values over 100% should fail.

* css3/filters/backdrop/backdropfilter-property-parsing-invalid-expected.txt:
* css3/filters/backdrop/backdropfilter-property-parsing-invalid.html:
* css3/filters/filter-property-parsing-expected.txt:
* css3/filters/filter-property-parsing-invalid-expected.txt:
* css3/filters/filter-property-parsing-invalid.html:
* css3/filters/filter-property-parsing.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@208294 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/css3/filters/backdrop/backdropfilter-property-parsing-invalid-expected.txt
LayoutTests/css3/filters/backdrop/backdropfilter-property-parsing-invalid.html
LayoutTests/css3/filters/filter-property-parsing-expected.txt
LayoutTests/css3/filters/filter-property-parsing-invalid-expected.txt
LayoutTests/css3/filters/filter-property-parsing-invalid.html
LayoutTests/css3/filters/filter-property-parsing.html
Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSParser.cpp

index 17184e4..0f5fc8a 100644 (file)
@@ -1,3 +1,20 @@
+2016-11-01  Dean Jackson  <dino@apple.com>
+
+        Filter functions grayscale/invert/opacity/sepia should clamp values over 100%, not fail
+        https://bugs.webkit.org/show_bug.cgi?id=164310
+        <rdar://problems/29057705>
+
+        Reviewed by Sam Weinig.
+
+        Some of our tests were incorrectly suggesting values over 100% should fail.
+
+        * css3/filters/backdrop/backdropfilter-property-parsing-invalid-expected.txt:
+        * css3/filters/backdrop/backdropfilter-property-parsing-invalid.html:
+        * css3/filters/filter-property-parsing-expected.txt:
+        * css3/filters/filter-property-parsing-invalid-expected.txt:
+        * css3/filters/filter-property-parsing-invalid.html:
+        * css3/filters/filter-property-parsing.html:
+
 2016-11-02  Brent Fulgham  <bfulgham@apple.com>
 
         WebKit nullptr dereference Archive Subframe
index 1255272..425259a 100644 (file)
@@ -39,11 +39,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
 
-Parameter out of bounds : grayscale(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
-
 Too many parameters : sepia(0.5 0.5 3.0)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -69,11 +64,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
 
-Parameter out of bounds : sepia(10000)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
-
 Length instead of number : saturate(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -154,11 +144,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
 
-Parameter out of bounds : invert(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
-
 Length instead of number : opacity(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -189,11 +174,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
 
-Parameter out of bounds : opacity(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-backdrop-filter') is ""
-
 Length instead of number : brightness(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
index 4f41d59..134f884 100644 (file)
@@ -38,14 +38,12 @@ testInvalidFilterRule("Too many parameters and commas", "grayscale(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "grayscale(0.5,)");
 testInvalidFilterRule("Negative parameter", "grayscale(-0.5)");
 testInvalidFilterRule("Negative percent", "grayscale(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "grayscale(1.5)");
 
 testInvalidFilterRule("Too many parameters", "sepia(0.5 0.5 3.0)");
 testInvalidFilterRule("Too many parameters and commas", "sepia(0.1, 0.1)");
 testInvalidFilterRule("Trailing comma", "sepia(0.5,)");
 testInvalidFilterRule("Negative parameter", "sepia(-0.01)");
 testInvalidFilterRule("Negative percent", "sepia(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "sepia(10000)");
 
 testInvalidFilterRule("Length instead of number", "saturate(10px)");
 testInvalidFilterRule("Too many parameters", "saturate(0.5 0.5)");
@@ -65,7 +63,6 @@ testInvalidFilterRule("Too many parameters", "invert(0.5 0.5)");
 testInvalidFilterRule("Too many parameters and commas", "invert(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "invert(0.5,)");
 testInvalidFilterRule("Negative parameter", "invert(-0.5)");
-testInvalidFilterRule("Parameter out of bounds", "invert(1.5)");
 
 testInvalidFilterRule("Length instead of number", "opacity(10px)");
 testInvalidFilterRule("Too many parameters", "opacity(0.5 0.5)");
@@ -73,7 +70,6 @@ testInvalidFilterRule("Too many parameters and commas", "opacity(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "opacity(0.5,)");
 testInvalidFilterRule("Negative parameter", "opacity(-0.5)");
 testInvalidFilterRule("Negative percent", "opacity(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "opacity(1.5)");
 
 testInvalidFilterRule("Length instead of number", "brightness(10px)");
 testInvalidFilterRule("Too many parameters", "brightness(0.5 0.5)");
index 8f98a70..9ca9c5c 100644 (file)
@@ -76,6 +76,26 @@ PASS jsWrapperClass(filterRule.constructor) is 'Function'
 PASS filterRule.length is 1
 PASS subRule.cssText is "grayscale(1)"
 
+Values over 1 are clamped : grayscale(1.5)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "grayscale(1)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "grayscale(1)"
+
+Percentages over 100 are clamped : grayscale(320%)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "grayscale(100%)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "grayscale(100%)"
+
 Zero value : grayscale(0)
 PASS cssRule.type is 1
 PASS declaration.length is 1
@@ -137,6 +157,26 @@ PASS jsWrapperClass(filterRule.constructor) is 'Function'
 PASS filterRule.length is 1
 PASS subRule.cssText is "sepia(1)"
 
+Values over 1 are clamped : sepia(8)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "sepia(1)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "sepia(1)"
+
+Percentages over 100 are clamped : sepia(101%)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "sepia(100%)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "sepia(100%)"
+
 Zero value : sepia(0)
 PASS cssRule.type is 1
 PASS declaration.length is 1
@@ -382,6 +422,26 @@ PASS jsWrapperClass(filterRule.constructor) is 'Function'
 PASS filterRule.length is 1
 PASS subRule.cssText is "invert(1)"
 
+Values over 1 are clamped : invert(1.01)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "invert(1)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "invert(1)"
+
+Percentages over 100 are clamped : invert(500000%)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "invert(100%)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "invert(100%)"
+
 Zero value : invert(0)
 PASS cssRule.type is 1
 PASS declaration.length is 1
@@ -454,6 +514,26 @@ PASS jsWrapperClass(filterRule.constructor) is 'Function'
 PASS filterRule.length is 1
 PASS subRule.cssText is "opacity(1)"
 
+Values over 1 are clamped : opacity(2134687326)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "opacity(1)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "opacity(1)"
+
+Percentages over 100 are clamped : opacity(500%)
+PASS cssRule.type is 1
+PASS declaration.length is 1
+PASS declaration.getPropertyValue('filter') is "opacity(100%)"
+PASS jsWrapperClass(filterRule) is 'CSSValueList'
+PASS jsWrapperClass(filterRule.__proto__) is 'CSSValueListPrototype'
+PASS jsWrapperClass(filterRule.constructor) is 'Function'
+PASS filterRule.length is 1
+PASS subRule.cssText is "opacity(100%)"
+
 Zero value : opacity(0)
 PASS cssRule.type is 1
 PASS declaration.length is 1
index f2b355e..5e77319 100644 (file)
@@ -39,11 +39,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-filter') is ""
 
-Parameter out of bounds : grayscale(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-filter') is ""
-
 Too many parameters : sepia(0.5 0.5 3.0)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -69,11 +64,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-filter') is ""
 
-Parameter out of bounds : sepia(10000)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-filter') is ""
-
 Length instead of number : saturate(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -154,11 +144,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-filter') is ""
 
-Parameter out of bounds : invert(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-filter') is ""
-
 Length instead of number : opacity(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
@@ -189,11 +174,6 @@ PASS cssRule.type is 1
 PASS declaration.length is 0
 PASS declaration.getPropertyValue('-webkit-filter') is ""
 
-Parameter out of bounds : opacity(1.5)
-PASS cssRule.type is 1
-PASS declaration.length is 0
-PASS declaration.getPropertyValue('-webkit-filter') is ""
-
 Length instead of number : brightness(10px)
 PASS cssRule.type is 1
 PASS declaration.length is 0
index 94bac57..84cc10f 100644 (file)
@@ -28,6 +28,7 @@ function testInvalidFilterRule(description, rule)
     declaration = cssRule.style;
     shouldBe("declaration.length", "0");
     shouldBeEqualToString("declaration.getPropertyValue('-webkit-filter')", "");
+    stylesheet.deleteRule(0);
 }
 
 testInvalidFilterRule("Too many parameters", "url(#a #b)");
@@ -38,14 +39,12 @@ testInvalidFilterRule("Too many parameters and commas", "grayscale(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "grayscale(0.5,)");
 testInvalidFilterRule("Negative parameter", "grayscale(-0.5)");
 testInvalidFilterRule("Negative percent", "grayscale(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "grayscale(1.5)");
 
 testInvalidFilterRule("Too many parameters", "sepia(0.5 0.5 3.0)");
 testInvalidFilterRule("Too many parameters and commas", "sepia(0.1, 0.1)");
 testInvalidFilterRule("Trailing comma", "sepia(0.5,)");
 testInvalidFilterRule("Negative parameter", "sepia(-0.01)");
 testInvalidFilterRule("Negative percent", "sepia(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "sepia(10000)");
 
 testInvalidFilterRule("Length instead of number", "saturate(10px)");
 testInvalidFilterRule("Too many parameters", "saturate(0.5 0.5)");
@@ -65,7 +64,6 @@ testInvalidFilterRule("Too many parameters", "invert(0.5 0.5)");
 testInvalidFilterRule("Too many parameters and commas", "invert(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "invert(0.5,)");
 testInvalidFilterRule("Negative parameter", "invert(-0.5)");
-testInvalidFilterRule("Parameter out of bounds", "invert(1.5)");
 
 testInvalidFilterRule("Length instead of number", "opacity(10px)");
 testInvalidFilterRule("Too many parameters", "opacity(0.5 0.5)");
@@ -73,7 +71,6 @@ testInvalidFilterRule("Too many parameters and commas", "opacity(0.5, 0.5)");
 testInvalidFilterRule("Trailing comma", "opacity(0.5,)");
 testInvalidFilterRule("Negative parameter", "opacity(-0.5)");
 testInvalidFilterRule("Negative percent", "opacity(-10%)");
-testInvalidFilterRule("Parameter out of bounds", "opacity(1.5)");
 
 testInvalidFilterRule("Length instead of number", "brightness(10px)");
 testInvalidFilterRule("Too many parameters", "brightness(0.5 0.5)");
index 7b679bf..70280d3 100644 (file)
@@ -90,6 +90,14 @@ testFilterRule("Float value converts to integer",
                "grayscale(1.0)", 1, "grayscale(1)",
                ["grayscale(1)"]);
 
+testFilterRule("Values over 1 are clamped",
+               "grayscale(1.5)", 1, "grayscale(1)",
+               ["grayscale(1)"]);
+
+testFilterRule("Percentages over 100 are clamped",
+               "grayscale(320%)", 1, "grayscale(100%)",
+               ["grayscale(100%)"]);
+
 testFilterRule("Zero value",
                "grayscale(0)", 1, "grayscale(0)",
                ["grayscale(0)"]);
@@ -114,6 +122,14 @@ testFilterRule("Float value converts to integer",
                "sepia(1.0)", 1, "sepia(1)",
                ["sepia(1)"]);
 
+testFilterRule("Values over 1 are clamped",
+               "sepia(8)", 1, "sepia(1)",
+               ["sepia(1)"]);
+
+testFilterRule("Percentages over 100 are clamped",
+               "sepia(101%)", 1, "sepia(100%)",
+               ["sepia(100%)"]);
+
 testFilterRule("Zero value",
                "sepia(0)", 1, "sepia(0)",
                ["sepia(0)"]);
@@ -210,6 +226,14 @@ testFilterRule("Float value converts to integer",
                "invert(1.0)", 1, "invert(1)",
                ["invert(1)"]);
 
+testFilterRule("Values over 1 are clamped",
+               "invert(1.01)", 1, "invert(1)",
+               ["invert(1)"]);
+
+testFilterRule("Percentages over 100 are clamped",
+               "invert(500000%)", 1, "invert(100%)",
+               ["invert(100%)"]);
+
 testFilterRule("Zero value",
                "invert(0)", 1, "invert(0)",
                ["invert(0)"]);
@@ -238,6 +262,14 @@ testFilterRule("Float value converts to integer",
                "opacity(1.0)", 1, "opacity(1)",
                ["opacity(1)"]);
 
+testFilterRule("Values over 1 are clamped",
+               "opacity(2134687326)", 1, "opacity(1)",
+               ["opacity(1)"]);
+
+testFilterRule("Percentages over 100 are clamped",
+               "opacity(500%)", 1, "opacity(100%)",
+               ["opacity(100%)"]);
+
 testFilterRule("Zero value",
                "opacity(0)", 1, "opacity(0)",
                ["opacity(0)"]);
index 9ec9c6c..f17a549 100644 (file)
@@ -1,3 +1,19 @@
+2016-11-01  Dean Jackson  <dino@apple.com>
+
+        Filter functions grayscale/invert/opacity/sepia should clamp values over 100%, not fail
+        https://bugs.webkit.org/show_bug.cgi?id=164310
+
+        Reviewed by Sam Weinig.
+
+        When bringing up the new CSS parser, I discovered that our old parser was
+        not conforming to the specification.
+
+        Covered by existing tests.
+
+        * css/parser/CSSParser.cpp:
+        (WebCore::CSSParser::parseBuiltinFilterArguments): For these functions, clamp to
+        100% rather than fail.
+
 2016-11-02  Brent Fulgham  <bfulgham@apple.com>
 
         WebKit nullptr dereference Archive Subframe
index 1368fad..cd3c73d 100644 (file)
@@ -10060,11 +10060,11 @@ RefPtr<CSSFunctionValue> CSSParser::parseBuiltinFilterArguments(CSSValueID filte
 
             auto primitiveValue = createPrimitiveNumericValue(argumentWithCalculation);
 
-            // Saturate and Contrast allow values over 100%.
+            // Saturate and contrast allow values over 100%. Otherwise clamp.
             if (filterFunction != CSSValueSaturate && filterFunction != CSSValueContrast) {
                 double maxAllowed = primitiveValue->primitiveType() == CSSPrimitiveValue::CSS_PERCENTAGE ? 100.0 : 1.0;
                 if (primitiveValue->doubleValue() > maxAllowed)
-                    return nullptr;
+                    primitiveValue = CSSValuePool::singleton().createValue(maxAllowed, static_cast<CSSPrimitiveValue::UnitTypes>(primitiveValue->primitiveType()));
             }
 
             filterValue->append(WTFMove(primitiveValue));