Handle filter() image type in new CSS Parser
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Nov 2016 19:57:03 +0000 (19:57 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Nov 2016 19:57:03 +0000 (19:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164673
<rdar://problems/29231376>

Reviewed by Darin Adler.

Source/WebCore:

Add support for the <image> type filter() to the
new CSS Parser. This involved moving some functions
from CSSPropertyParser into CSSPropertyParserHelpers.

Covered by fast/filter-image tests.

* css/CSSValueKeywords.in:
* css/parser/CSSPropertyParser.cpp:
(WebCore::consumeShadow):
(WebCore::parseSingleShadow): Deleted.
(WebCore::consumeFilterFunction): Deleted.
(WebCore::consumeFilter): Deleted.
* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeFilterImage):
(WebCore::CSSPropertyParserHelpers::consumeGeneratedImage):
(WebCore::CSSPropertyParserHelpers::isGeneratedImage):
(WebCore::CSSPropertyParserHelpers::consumeFilterFunction):
(WebCore::CSSPropertyParserHelpers::consumeFilter):
(WebCore::CSSPropertyParserHelpers::consumeSingleShadow):
* css/parser/CSSPropertyParserHelpers.h:

LayoutTests:

Remove the test that allows a string value for the
image source rather than a url().

* fast/filter-image/filter-image-animation-expected.txt:
* fast/filter-image/filter-image-animation.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/filter-image/filter-image-animation-expected.txt
LayoutTests/fast/filter-image/filter-image-animation.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSValueKeywords.in
Source/WebCore/css/parser/CSSPropertyParser.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.h

index 0acb7b1..9f11fee 100644 (file)
@@ -1,3 +1,17 @@
+2016-11-14  Dean Jackson  <dino@apple.com>
+
+        Handle filter() image type in new CSS Parser
+        https://bugs.webkit.org/show_bug.cgi?id=164673
+        <rdar://problems/29231376>
+
+        Reviewed by Darin Adler.
+
+        Remove the test that allows a string value for the
+        image source rather than a url().
+
+        * fast/filter-image/filter-image-animation-expected.txt:
+        * fast/filter-image/filter-image-animation.html:
+
 2016-11-14  Joanmarie Diggs  <jdiggs@igalia.com>
 
         AX: In ARIA 1.1, the implicit value for aria-level on headings is 2
index 6118ccc..f53ee5d 100644 (file)
@@ -1,10 +1,9 @@
-            
+           
 PASS - "backgroundImage" property for "brightness" element at 1s saw something close to: filter(url(image.svg), brightness(0.5))
 PASS - "backgroundImage" property for "blur" element at 1s saw something close to: filter(url(image.svg), blur(5px))
 PASS - "backgroundImage" property for "grayscale" element at 1s saw something close to: filter(url(image.svg), grayscale(0.5))
 PASS - "backgroundImage" property for "sepia" element at 1s saw something close to: filter(url(image.svg), sepia(0.5))
 PASS - "backgroundImage" property for "no" element at 1s saw something close to: filter(url(blue.svg), sepia(0))
-PASS - "backgroundImage" property for "string" element at 1s saw something close to: filter(url(image.svg), sepia(0.5))
 PASS - "backgroundImage" property for "multiple1" element at 1s saw something close to: filter(url(image.svg), sepia(0.5) blur(1.5px) hue-rotate(22.5deg))
 PASS - "backgroundImage" property for "multiple2" element at 1s saw something close to: filter(url(image.svg), contrast(0.5) blur(1.5px) hue-rotate(22.5deg))
 PASS - "backgroundImage" property for "urlfilter" element at 1s saw something close to: filter(url(image.svg), sepia(0.25))
index 262230c..6307bab 100644 (file)
       -webkit-animation: no-anim 2s linear;
     }
 
-    #string {
-      -webkit-animation: string-anim 2s linear;
-    }
-
     #multiple1 {
       -webkit-animation: multiple-anim1 2s linear;
     }
         to   { background-image: filter(url(resources/blue.svg), sepia(0)); }
     }
 
-    @-webkit-keyframes string-anim {
-        from { background-image: filter('resources/image.svg', sepia(1)); }
-        to   { background-image: filter(url(resources/image.svg), sepia(0)); }
-    }
-
     @-webkit-keyframes multiple-anim1 {
         from { background-image: filter(url(resources/image.svg), sepia(0.25) blur(3px) hue-rotate(45deg)); }
         to   { background-image: filter(url(resources/image.svg), sepia(0.75)); }
       ["grayscale-anim",  1, "grayscale", "backgroundImage", "filter(url(image.svg), grayscale(0.5))", 0.05],
       ["sepia-anim",  1, "sepia", "backgroundImage", "filter(url(image.svg), sepia(0.5))", 0.05],
       ["no-anim",  1, "no", "backgroundImage", "filter(url(blue.svg), sepia(0))", 0],
-      ["string-anim",  1, "string", "backgroundImage", "filter(url(image.svg), sepia(0.5))", 0],
       ["multiple-anim1",  1, "multiple1", "backgroundImage", "filter(url(image.svg), sepia(0.5) blur(1.5px) hue-rotate(22.5deg))", 0.05],
       ["multiple-anim2",  1, "multiple2", "backgroundImage", "filter(url(image.svg), contrast(0.5) blur(1.5px) hue-rotate(22.5deg))", 0.05],
       ["url-to-filter-anim",  1, "urlfilter", "backgroundImage", " filter(url(image.svg), sepia(0.25))", 0.05],
 <div class="box" id="grayscale"></div>
 <div class="box" id="sepia"></div>
 <div class="box" id="no"></div>
-<div class="box" id="string"></div>
 <div class="box" id="multiple1"></div>
 <div class="box" id="multiple2"></div>
 <div class="box" id="urlfilter"></div>
index 7b2caa8..2fbc52e 100644 (file)
@@ -1,3 +1,32 @@
+2016-11-14  Dean Jackson  <dino@apple.com>
+
+        Handle filter() image type in new CSS Parser
+        https://bugs.webkit.org/show_bug.cgi?id=164673
+        <rdar://problems/29231376>
+
+        Reviewed by Darin Adler.
+
+        Add support for the <image> type filter() to the
+        new CSS Parser. This involved moving some functions
+        from CSSPropertyParser into CSSPropertyParserHelpers.
+
+        Covered by fast/filter-image tests.
+
+        * css/CSSValueKeywords.in:
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::consumeShadow):
+        (WebCore::parseSingleShadow): Deleted.
+        (WebCore::consumeFilterFunction): Deleted.
+        (WebCore::consumeFilter): Deleted.
+        * css/parser/CSSPropertyParserHelpers.cpp:
+        (WebCore::CSSPropertyParserHelpers::consumeFilterImage):
+        (WebCore::CSSPropertyParserHelpers::consumeGeneratedImage):
+        (WebCore::CSSPropertyParserHelpers::isGeneratedImage):
+        (WebCore::CSSPropertyParserHelpers::consumeFilterFunction):
+        (WebCore::CSSPropertyParserHelpers::consumeFilter):
+        (WebCore::CSSPropertyParserHelpers::consumeSingleShadow):
+        * css/parser/CSSPropertyParserHelpers.h:
+
 2016-11-14  Dave Hyatt  <hyatt@apple.com>
 
         [CSS Parser] Support percentages in word-spacing
index c1c052a..b69aec2 100644 (file)
@@ -1165,6 +1165,8 @@ repeating-radial-gradient
 -webkit-repeating-radial-gradient
 -webkit-image-set
 -webkit-named-image
+filter
+-webkit-filter
 
 // deprecated gradients
 from
index 906b48f..0d6cf3f 100644 (file)
@@ -1353,52 +1353,6 @@ static RefPtr<CSSValue> consumeZIndex(CSSParserTokenRange& range)
     return consumeInteger(range);
 }
 
-static RefPtr<CSSShadowValue> parseSingleShadow(CSSParserTokenRange& range, CSSParserMode cssParserMode, bool allowInset, bool allowSpread)
-{
-    RefPtr<CSSPrimitiveValue> style;
-    RefPtr<CSSPrimitiveValue> color;
-
-    if (range.atEnd())
-        return nullptr;
-    if (range.peek().id() == CSSValueInset) {
-        if (!allowInset)
-            return nullptr;
-        style = consumeIdent(range);
-    }
-    color = consumeColor(range, cssParserMode);
-
-    RefPtr<CSSPrimitiveValue> horizontalOffset = consumeLength(range, cssParserMode, ValueRangeAll);
-    if (!horizontalOffset)
-        return nullptr;
-
-    RefPtr<CSSPrimitiveValue> verticalOffset = consumeLength(range, cssParserMode, ValueRangeAll);
-    if (!verticalOffset)
-        return nullptr;
-
-    RefPtr<CSSPrimitiveValue> blurRadius = consumeLength(range, cssParserMode, ValueRangeAll);
-    RefPtr<CSSPrimitiveValue> spreadDistance;
-    if (blurRadius) {
-        // Blur radius must be non-negative.
-        if (blurRadius->doubleValue() < 0)
-            return nullptr;
-        if (allowSpread)
-            spreadDistance = consumeLength(range, cssParserMode, ValueRangeAll);
-    }
-
-    if (!range.atEnd()) {
-        if (!color)
-            color = consumeColor(range, cssParserMode);
-        if (range.peek().id() == CSSValueInset) {
-            if (!allowInset || style)
-                return nullptr;
-            style = consumeIdent(range);
-        }
-    }
-    
-    // We pass RefPtrs, since they can actually be null.
-    return CSSShadowValue::create(WTFMove(horizontalOffset), WTFMove(verticalOffset), WTFMove(blurRadius), WTFMove(spreadDistance), WTFMove(style), WTFMove(color));
-}
-
 static RefPtr<CSSValue> consumeShadow(CSSParserTokenRange& range, CSSParserMode cssParserMode, bool isBoxShadowProperty)
 {
     if (range.peek().id() == CSSValueNone)
@@ -1406,7 +1360,7 @@ static RefPtr<CSSValue> consumeShadow(CSSParserTokenRange& range, CSSParserMode
 
     RefPtr<CSSValueList> shadowValueList = CSSValueList::createCommaSeparated();
     do {
-        if (RefPtr<CSSShadowValue> shadowValue = parseSingleShadow(range, cssParserMode, isBoxShadowProperty, isBoxShadowProperty))
+        if (RefPtr<CSSShadowValue> shadowValue = consumeSingleShadow(range, cssParserMode, isBoxShadowProperty, isBoxShadowProperty))
             shadowValueList->append(*shadowValue);
         else
             return nullptr;
@@ -1414,67 +1368,6 @@ static RefPtr<CSSValue> consumeShadow(CSSParserTokenRange& range, CSSParserMode
     return shadowValueList;
 }
 
-static RefPtr<CSSFunctionValue> consumeFilterFunction(CSSParserTokenRange& range, const CSSParserContext& context)
-{
-    CSSValueID filterType = range.peek().functionId();
-    if (filterType < CSSValueInvert || filterType > CSSValueDropShadow)
-        return nullptr;
-    CSSParserTokenRange args = consumeFunction(range);
-    RefPtr<CSSFunctionValue> filterValue = CSSFunctionValue::create(filterType);
-    RefPtr<CSSValue> parsedValue;
-
-    if (filterType == CSSValueDropShadow)
-        parsedValue = parseSingleShadow(args, context.mode, false, false);
-    else {
-        if (args.atEnd())
-            return filterValue;
-        if (filterType == CSSValueBrightness) {
-            // FIXME (crbug.com/397061): Support calc expressions like calc(10% + 0.5)
-            parsedValue = consumePercent(args, ValueRangeAll);
-            if (!parsedValue)
-                parsedValue = consumeNumber(args, ValueRangeAll);
-        } else if (filterType == CSSValueHueRotate)
-            parsedValue = consumeAngle(args, context.mode, UnitlessQuirk::Forbid);
-        else if (filterType == CSSValueBlur) {
-            parsedValue = consumeLength(args, HTMLStandardMode, ValueRangeNonNegative);
-        } else {
-            // FIXME (crbug.com/397061): Support calc expressions like calc(10% + 0.5)
-            parsedValue = consumePercent(args, ValueRangeNonNegative);
-            if (!parsedValue)
-                parsedValue = consumeNumber(args, ValueRangeNonNegative);
-            if (parsedValue && filterType != CSSValueSaturate && filterType != CSSValueContrast) {
-                bool isPercentage = downcast<CSSPrimitiveValue>(*parsedValue).isPercentage();
-                double maxAllowed = isPercentage ? 100.0 : 1.0;
-                if (downcast<CSSPrimitiveValue>(*parsedValue).doubleValue() > maxAllowed) {
-                    parsedValue = CSSPrimitiveValue::create(maxAllowed, isPercentage ? CSSPrimitiveValue::UnitTypes::CSS_PERCENTAGE : CSSPrimitiveValue::UnitTypes::CSS_NUMBER);
-                }
-            }
-        }
-    }
-    if (!parsedValue || !args.atEnd())
-        return nullptr;
-    filterValue->append(*parsedValue);
-    return filterValue;
-}
-
-static RefPtr<CSSValue> consumeFilter(CSSParserTokenRange& range, const CSSParserContext& context)
-{
-    if (range.peek().id() == CSSValueNone)
-        return consumeIdent(range);
-
-    RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
-    do {
-        RefPtr<CSSValue> filterValue = consumeUrl(range);
-        if (!filterValue) {
-            filterValue = consumeFilterFunction(range, context);
-            if (!filterValue)
-                return nullptr;
-        }
-        list->append(filterValue.releaseNonNull());
-    } while (!range.atEnd());
-    return list;
-}
-
 static RefPtr<CSSValue> consumeTextDecorationLine(CSSParserTokenRange& range)
 {
     CSSValueID id = range.peek().id();
index 7111e34..cc9c86a 100644 (file)
@@ -33,6 +33,7 @@
 #include "CSSCalculationValue.h"
 #include "CSSCanvasValue.h"
 #include "CSSCrossfadeValue.h"
+#include "CSSFilterImageValue.h"
 #include "CSSGradientValue.h"
 #include "CSSImageSetValue.h"
 #include "CSSImageValue.h"
@@ -1086,7 +1087,24 @@ static RefPtr<CSSValue> consumeWebkitNamedImage(CSSParserTokenRange& args)
         return nullptr;
     return CSSNamedImageValue::create(imageName);
 }
-    
+
+static RefPtr<CSSValue> consumeFilterImage(CSSParserTokenRange& args, const CSSParserContext& context)
+{
+    auto imageValue = consumeImageOrNone(args, context);
+    if (!imageValue || !consumeCommaIncludingWhitespace(args))
+        return nullptr;
+
+    auto filterValue = consumeFilter(args, context);
+
+    if (!filterValue)
+        return nullptr;
+
+    if (!args.atEnd())
+        return nullptr;
+
+    return CSSFilterImageValue::create(imageValue.releaseNonNull(), filterValue.releaseNonNull());
+}
+
 static RefPtr<CSSValue> consumeGeneratedImage(CSSParserTokenRange& range, CSSParserContext context)
 {
     CSSValueID id = range.peek().functionId();
@@ -1117,7 +1135,8 @@ static RefPtr<CSSValue> consumeGeneratedImage(CSSParserTokenRange& range, CSSPar
         result = consumeWebkitCanvas(args);
     else if (id == CSSValueWebkitNamedImage)
         result = consumeWebkitNamedImage(args);
-
+    else if (id == CSSValueWebkitFilter || id == CSSValueFilter)
+        result = consumeFilterImage(args, context);
     if (!result || !args.atEnd())
         return nullptr;
     range = rangeCopy;
@@ -1161,7 +1180,111 @@ static bool isGeneratedImage(CSSValueID id)
         || id == CSSValueWebkitLinearGradient || id == CSSValueWebkitRadialGradient
         || id == CSSValueWebkitRepeatingLinearGradient || id == CSSValueWebkitRepeatingRadialGradient
         || id == CSSValueWebkitGradient || id == CSSValueWebkitCrossFade || id == CSSValueWebkitCanvas
-        || id == CSSValueCrossFade || id == CSSValueWebkitNamedImage;
+        || id == CSSValueCrossFade || id == CSSValueWebkitNamedImage || id == CSSValueWebkitFilter || id == CSSValueFilter;
+}
+
+RefPtr<CSSFunctionValue> consumeFilterFunction(CSSParserTokenRange& range, const CSSParserContext& context)
+{
+    CSSValueID filterType = range.peek().functionId();
+    if (filterType < CSSValueInvert || filterType > CSSValueDropShadow)
+        return nullptr;
+    CSSParserTokenRange args = consumeFunction(range);
+    auto filterValue = CSSFunctionValue::create(filterType);
+    RefPtr<CSSValue> parsedValue;
+
+    if (filterType == CSSValueDropShadow)
+        parsedValue = consumeSingleShadow(args, context.mode, false, false);
+    else {
+        if (args.atEnd())
+            return filterValue.ptr();
+        if (filterType == CSSValueBrightness) {
+            parsedValue = consumePercent(args, ValueRangeAll);
+            if (!parsedValue)
+                parsedValue = consumeNumber(args, ValueRangeAll);
+        } else if (filterType == CSSValueHueRotate)
+            parsedValue = consumeAngle(args, context.mode, UnitlessQuirk::Forbid);
+        else if (filterType == CSSValueBlur)
+            parsedValue = consumeLength(args, HTMLStandardMode, ValueRangeNonNegative);
+        else {
+            parsedValue = consumePercent(args, ValueRangeNonNegative);
+            if (!parsedValue)
+                parsedValue = consumeNumber(args, ValueRangeNonNegative);
+            if (parsedValue && filterType != CSSValueSaturate && filterType != CSSValueContrast) {
+                bool isPercentage = downcast<CSSPrimitiveValue>(*parsedValue).isPercentage();
+                double maxAllowed = isPercentage ? 100.0 : 1.0;
+                if (downcast<CSSPrimitiveValue>(*parsedValue).doubleValue() > maxAllowed)
+                    parsedValue = CSSPrimitiveValue::create(maxAllowed, isPercentage ? CSSPrimitiveValue::UnitTypes::CSS_PERCENTAGE : CSSPrimitiveValue::UnitTypes::CSS_NUMBER);
+            }
+        }
+    }
+    if (!parsedValue || !args.atEnd())
+        return nullptr;
+    filterValue->append(*parsedValue);
+    return filterValue.ptr();
+}
+
+RefPtr<CSSValue> consumeFilter(CSSParserTokenRange& range, const CSSParserContext& context)
+{
+    if (range.peek().id() == CSSValueNone)
+        return consumeIdent(range);
+
+    auto list = CSSValueList::createSpaceSeparated();
+    do {
+        RefPtr<CSSValue> filterValue = consumeUrl(range);
+        if (!filterValue) {
+            filterValue = consumeFilterFunction(range, context);
+            if (!filterValue)
+                return nullptr;
+        }
+        list->append(filterValue.releaseNonNull());
+    } while (!range.atEnd());
+
+    return list.ptr();
+}
+
+RefPtr<CSSShadowValue> consumeSingleShadow(CSSParserTokenRange& range, CSSParserMode cssParserMode, bool allowInset, bool allowSpread)
+{
+    RefPtr<CSSPrimitiveValue> style;
+    RefPtr<CSSPrimitiveValue> color;
+
+    if (range.atEnd())
+        return nullptr;
+    if (range.peek().id() == CSSValueInset) {
+        if (!allowInset)
+            return nullptr;
+        style = consumeIdent(range);
+    }
+    color = consumeColor(range, cssParserMode);
+
+    auto horizontalOffset = consumeLength(range, cssParserMode, ValueRangeAll);
+    if (!horizontalOffset)
+        return nullptr;
+
+    auto verticalOffset = consumeLength(range, cssParserMode, ValueRangeAll);
+    if (!verticalOffset)
+        return nullptr;
+
+    auto blurRadius = consumeLength(range, cssParserMode, ValueRangeAll);
+    RefPtr<CSSPrimitiveValue> spreadDistance;
+    if (blurRadius) {
+        // Blur radius must be non-negative.
+        if (blurRadius->doubleValue() < 0)
+            return nullptr;
+        if (allowSpread)
+            spreadDistance = consumeLength(range, cssParserMode, ValueRangeAll);
+    }
+
+    if (!range.atEnd()) {
+        if (!color)
+            color = consumeColor(range, cssParserMode);
+        if (range.peek().id() == CSSValueInset) {
+            if (!allowInset || style)
+                return nullptr;
+            style = consumeIdent(range);
+        }
+    }
+
+    return CSSShadowValue::create(WTFMove(horizontalOffset), WTFMove(verticalOffset), WTFMove(blurRadius), WTFMove(spreadDistance), WTFMove(style), WTFMove(color));
 }
 
 RefPtr<CSSValue> consumeImage(CSSParserTokenRange& range, CSSParserContext context, ConsumeGeneratedImage generatedImage)
index 671ec03..a9ea89b 100644 (file)
 #pragma once
 
 #include "CSSCustomIdentValue.h"
+#include "CSSFunctionValue.h"
 #include "CSSParserMode.h"
 #include "CSSParserTokenRange.h"
 #include "CSSPrimitiveValue.h"
+#include "CSSShadowValue.h"
 #include "Length.h" // For ValueRange
 
 namespace WebCore {
@@ -88,6 +90,10 @@ enum class ConsumeGeneratedImage {
 RefPtr<CSSValue> consumeImage(CSSParserTokenRange&, CSSParserContext, ConsumeGeneratedImage = ConsumeGeneratedImage::Allow);
 RefPtr<CSSValue> consumeImageOrNone(CSSParserTokenRange&, CSSParserContext);
 
+RefPtr<CSSValue> consumeFilter(CSSParserTokenRange&, const CSSParserContext&);
+RefPtr<CSSFunctionValue> consumeFilterFunction(CSSParserTokenRange&, const CSSParserContext&);
+RefPtr<CSSShadowValue> consumeSingleShadow(CSSParserTokenRange&, CSSParserMode, bool allowInset, bool allowSpread);
+
 // Template implementations are at the bottom of the file for readability.
 
 template<typename... emptyBaseCase> inline bool identMatches(CSSValueID) { return false; }