Title: [288134] trunk
Revision
288134
Author
wei...@apple.com
Date
2022-01-18 11:59:53 -0800 (Tue, 18 Jan 2022)

Log Message

Canvas functions that take colors as strings don't support all the syntax that CSS supports
https://bugs.webkit.org/show_bug.cgi?id=235269

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update expected results now that the test is passing.

* web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.toStringFunctionCallback-expected.txt:

Source/WebCore:

Add a variant of CSSParser::parseColor() that takes a CSSParserContext
and use it (and parser context created from a document) to get the parser
to respect the settings. Rename the existing CSSParser::parseColor() to
CSSParser::parseColorWithoutContext() and add a comment indicating all
uses should eventually be removed.

Offscreen canvas and custom paint canvas are using the old path with https://webkit.org/b/235270
tracking finding a solution for them.

* Modules/applicationmanifest/ApplicationManifestParser.cpp:
(WebCore::ApplicationManifestParser::parseColor):
* css/StyleProperties.cpp:
(WebCore::StyleProperties::propertyAsColor const):
* css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseColor):
(WebCore::CSSParser::parseColorWithoutContext):
* css/parser/CSSParser.h:
* editing/EditingStyle.cpp:
(WebCore::cssValueToColor):
* html/HTMLBodyElement.cpp:
(WebCore::HTMLBodyElement::parseAttribute):
* html/HTMLMetaElement.cpp:
(WebCore::HTMLMetaElement::contentColor):
* html/canvas/CanvasStyle.cpp:
(WebCore::parseColor):
(WebCore::currentColor):
* svg/properties/SVGAnimationAdditiveValueFunctionImpl.h:
* svg/properties/SVGPropertyTraits.h:
(WebCore::SVGPropertyTraits<Color>::fromString):
(WebCore::SVGPropertyTraits<Color>::parse):

Source/WebKit:

Update for rename of CSSParser::parseColor() to CSSParser::parseColorWithoutContext().

* UIProcess/API/wpe/WebKitColor.cpp:
(webkit_color_parse):
* WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageSetComposition):

LayoutTests:

* fast/canvas/canvas-color-serialization.html:
Update doctype to set the correct CSS parsing mode which now needs to be set correctly for canvas color setting functions.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (288133 => 288134)


--- trunk/LayoutTests/ChangeLog	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/LayoutTests/ChangeLog	2022-01-18 19:59:53 UTC (rev 288134)
@@ -1,3 +1,13 @@
+2022-01-18  Sam Weinig  <wei...@apple.com>
+
+        Canvas functions that take colors as strings don't support all the syntax that CSS supports
+        https://bugs.webkit.org/show_bug.cgi?id=235269
+
+        Reviewed by Darin Adler.
+
+        * fast/canvas/canvas-color-serialization.html:
+        Update doctype to set the correct CSS parsing mode which now needs to be set correctly for canvas color setting functions.
+
 2022-01-18  Patrick Griffis  <pgrif...@igalia.com>
 
         CSP: Improve handling of multiple policies

Modified: trunk/LayoutTests/fast/canvas/canvas-color-serialization.html (288133 => 288134)


--- trunk/LayoutTests/fast/canvas/canvas-color-serialization.html	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/LayoutTests/fast/canvas/canvas-color-serialization.html	2022-01-18 19:59:53 UTC (rev 288134)
@@ -1,4 +1,4 @@
-<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<!DOCTYPE HTML>
 <html>
 <head>
 <script src=""

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (288133 => 288134)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2022-01-18 19:59:53 UTC (rev 288134)
@@ -1,3 +1,14 @@
+2022-01-18  Sam Weinig  <wei...@apple.com>
+
+        Canvas functions that take colors as strings don't support all the syntax that CSS supports
+        https://bugs.webkit.org/show_bug.cgi?id=235269
+
+        Reviewed by Darin Adler.
+
+        Update expected results now that the test is passing.
+
+        * web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.toStringFunctionCallback-expected.txt:
+
 2022-01-18  Patrick Griffis  <pgrif...@igalia.com>
 
         CSP: Improve handling of multiple policies

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.toStringFunctionCallback-expected.txt (288133 => 288134)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.toStringFunctionCallback-expected.txt	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.toStringFunctionCallback-expected.txt	2022-01-18 19:59:53 UTC (rev 288134)
@@ -2,5 +2,5 @@
 Passing a function in to ctx.fillStyle or ctx.strokeStyle with a toString callback works as specified
 Actual output:
 
-FAIL Passing a function in to ctx.fillStyle or ctx.strokeStyle with a toString callback works as specified assert_equals: ctx.fillStyle === "#008000" (got #800000[string], expected #008000[string]) expected "#008000" but got "#800000"
+PASS Passing a function in to ctx.fillStyle or ctx.strokeStyle with a toString callback works as specified
 

Modified: trunk/Source/WebCore/ChangeLog (288133 => 288134)


--- trunk/Source/WebCore/ChangeLog	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/ChangeLog	2022-01-18 19:59:53 UTC (rev 288134)
@@ -1,3 +1,41 @@
+2022-01-18  Sam Weinig  <wei...@apple.com>
+
+        Canvas functions that take colors as strings don't support all the syntax that CSS supports
+        https://bugs.webkit.org/show_bug.cgi?id=235269
+
+        Reviewed by Darin Adler.
+
+        Add a variant of CSSParser::parseColor() that takes a CSSParserContext
+        and use it (and parser context created from a document) to get the parser
+        to respect the settings. Rename the existing CSSParser::parseColor() to 
+        CSSParser::parseColorWithoutContext() and add a comment indicating all
+        uses should eventually be removed.
+
+        Offscreen canvas and custom paint canvas are using the old path with https://webkit.org/b/235270
+        tracking finding a solution for them.
+
+        * Modules/applicationmanifest/ApplicationManifestParser.cpp:
+        (WebCore::ApplicationManifestParser::parseColor):
+        * css/StyleProperties.cpp:
+        (WebCore::StyleProperties::propertyAsColor const):
+        * css/parser/CSSParser.cpp:
+        (WebCore::CSSParser::parseColor):
+        (WebCore::CSSParser::parseColorWithoutContext):
+        * css/parser/CSSParser.h:
+        * editing/EditingStyle.cpp:
+        (WebCore::cssValueToColor):
+        * html/HTMLBodyElement.cpp:
+        (WebCore::HTMLBodyElement::parseAttribute):
+        * html/HTMLMetaElement.cpp:
+        (WebCore::HTMLMetaElement::contentColor):
+        * html/canvas/CanvasStyle.cpp:
+        (WebCore::parseColor):
+        (WebCore::currentColor):
+        * svg/properties/SVGAnimationAdditiveValueFunctionImpl.h:
+        * svg/properties/SVGPropertyTraits.h:
+        (WebCore::SVGPropertyTraits<Color>::fromString):
+        (WebCore::SVGPropertyTraits<Color>::parse):
+
 2022-01-18  Alex Christensen  <achristen...@webkit.org>
 
         Remove ImplementationLacksVTable IDL attribute

Modified: trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp (288133 => 288134)


--- trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -320,7 +320,7 @@
 
 Color ApplicationManifestParser::parseColor(const JSON::Object& manifest, const String& propertyName)
 {
-    return CSSParser::parseColor(parseGenericString(manifest, propertyName));
+    return CSSParser::parseColorWithoutContext(parseGenericString(manifest, propertyName));
 }
 
 String ApplicationManifestParser::parseGenericString(const JSON::Object& manifest, const String& propertyName)

Modified: trunk/Source/WebCore/css/StyleProperties.cpp (288133 => 288134)


--- trunk/Source/WebCore/css/StyleProperties.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/css/StyleProperties.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -347,7 +347,7 @@
         return std::nullopt;
 
     auto& primitiveColor = downcast<CSSPrimitiveValue>(*colorValue);
-    return primitiveColor.isRGBColor() ? primitiveColor.color() : CSSParser::parseColor(colorValue->cssText());
+    return primitiveColor.isRGBColor() ? primitiveColor.color() : CSSParser::parseColorWithoutContext(colorValue->cssText());
 }
 
 CSSValueID StyleProperties::propertyAsValueID(CSSPropertyID property) const

Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (288133 => 288134)


--- trunk/Source/WebCore/css/parser/CSSParser.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -94,10 +94,24 @@
     return CSSSupportsParser::supportsCondition(parser.tokenizer()->tokenRange(), parser, CSSSupportsParser::ForWindowCSS) == CSSSupportsParser::Supported;
 }
 
-Color CSSParser::parseColor(const String& string, bool strict)
+Color CSSParser::parseColor(const String& string, const CSSParserContext& context)
 {
+    bool strict = !isQuirksModeBehavior(context.mode);
     if (auto color = CSSParserFastPaths::parseSimpleColor(string, strict))
         return *color;
+    auto value = parseSingleValue(CSSPropertyColor, string, context);
+    if (!is<CSSPrimitiveValue>(value))
+        return { };
+    auto& primitiveValue = downcast<CSSPrimitiveValue>(*value);
+    if (!primitiveValue.isRGBColor())
+        return { };
+    return primitiveValue.color();
+}
+
+Color CSSParser::parseColorWithoutContext(const String& string, bool strict)
+{
+    if (auto color = CSSParserFastPaths::parseSimpleColor(string, strict))
+        return *color;
     // FIXME: Unclear why we want to ignore the boolean argument "strict" and always pass strictCSSParserContext here.
     auto value = parseSingleValue(CSSPropertyColor, string, strictCSSParserContext());
     if (!is<CSSPrimitiveValue>(value))

Modified: trunk/Source/WebCore/css/parser/CSSParser.h (288133 => 288134)


--- trunk/Source/WebCore/css/parser/CSSParser.h	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/css/parser/CSSParser.h	2022-01-18 19:59:53 UTC (rev 288134)
@@ -88,7 +88,9 @@
 
     RefPtr<CSSValue> parseValueWithVariableReferences(CSSPropertyID, const CSSValue&, Style::BuilderState&);
 
-    WEBCORE_EXPORT static Color parseColor(const String&, bool strict = false);
+    WEBCORE_EXPORT static Color parseColor(const String&, const CSSParserContext&);
+    // FIXME: All callers are not getting the right Settings for parsing due to lack of CSSParserContext and should switch to the parseColor function above.
+    WEBCORE_EXPORT static Color parseColorWithoutContext(const String&, bool strict = false);
     static Color parseSystemColor(StringView);
     static std::optional<SRGBA<uint8_t>> parseNamedColor(StringView);
     static std::optional<SRGBA<uint8_t>> parseHexColor(StringView);

Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (288133 => 288134)


--- trunk/Source/WebCore/editing/EditingStyle.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -439,7 +439,7 @@
     if (primitiveColor.isRGBColor())
         return primitiveColor.color();
     
-    return CSSParser::parseColor(colorValue->cssText());
+    return CSSParser::parseColorWithoutContext(colorValue->cssText());
 }
 
 template<typename T>

Modified: trunk/Source/WebCore/html/HTMLBodyElement.cpp (288133 => 288134)


--- trunk/Source/WebCore/html/HTMLBodyElement.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/html/HTMLBodyElement.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -123,7 +123,7 @@
             else
                 document().resetActiveLinkColor();
         } else {
-            Color color = CSSParser::parseColor(value, !document().inQuirksMode());
+            Color color = CSSParser::parseColorWithoutContext(value, !document().inQuirksMode());
             if (color.isValid()) {
                 if (name == linkAttr)
                     document().setLinkColor(color);

Modified: trunk/Source/WebCore/html/HTMLMetaElement.cpp (288133 => 288134)


--- trunk/Source/WebCore/html/HTMLMetaElement.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/html/HTMLMetaElement.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -85,7 +85,7 @@
 const Color& HTMLMetaElement::contentColor()
 {
     if (!m_contentColor)
-        m_contentColor = CSSParser::parseColor(content());
+        m_contentColor = CSSParser::parseColorWithoutContext(content());
     return *m_contentColor;
 }
 

Modified: trunk/Source/WebCore/html/canvas/CanvasStyle.cpp (288133 => 288134)


--- trunk/Source/WebCore/html/canvas/CanvasStyle.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/html/canvas/CanvasStyle.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -44,10 +44,6 @@
 #include "OffscreenCanvas.h"
 #endif
 
-#if USE(CG)
-#include <CoreGraphics/CGContext.h>
-#endif
-
 namespace WebCore {
 
 bool isCurrentColorString(const String& colorString)
@@ -58,12 +54,16 @@
 Color parseColor(const String& colorString, CanvasBase& canvasBase)
 {
 #if ENABLE(OFFSCREEN_CANVAS)
-    if (canvasBase.isOffscreenCanvas())
+    if (is<OffscreenCanvas>(canvasBase))
         return CSSPropertyParserWorkerSafe::parseColor(colorString);
-#else
-    UNUSED_PARAM(canvasBase);
 #endif
-    Color color = CSSParser::parseColor(colorString);
+
+    Color color;
+    if (is<HTMLCanvasElement>(canvasBase))
+        color = CSSParser::parseColor(colorString, CSSParserContext { downcast<HTMLCanvasElement>(canvasBase).document() });
+    else
+        color = CSSParser::parseColorWithoutContext(colorString);
+
     if (color.isValid())
         return color;
     return CSSParser::parseSystemColor(colorString);
@@ -77,7 +77,7 @@
     auto& canvas = downcast<HTMLCanvasElement>(canvasBase);
     if (!canvas.isConnected() || !canvas.inlineStyle())
         return Color::black;
-    Color color = CSSParser::parseColor(canvas.inlineStyle()->getPropertyValue(CSSPropertyColor));
+    Color color = CSSParser::parseColorWithoutContext(canvas.inlineStyle()->getPropertyValue(CSSPropertyColor));
     if (!color.isValid())
         return Color::black;
     return color;

Modified: trunk/Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.h (288133 => 288134)


--- trunk/Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.h	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.h	2022-01-18 19:59:53 UTC (rev 288134)
@@ -96,10 +96,10 @@
 
     std::optional<float> calculateDistance(SVGElement&, const String& from, const String& to) const override
     {
-        Color fromColor = CSSParser::parseColor(from.stripWhiteSpace());
+        Color fromColor = CSSParser::parseColorWithoutContext(from.stripWhiteSpace());
         if (!fromColor.isValid())
             return { };
-        Color toColor = CSSParser::parseColor(to.stripWhiteSpace());
+        Color toColor = CSSParser::parseColorWithoutContext(to.stripWhiteSpace());
         if (!toColor.isValid())
             return { };
             

Modified: trunk/Source/WebCore/svg/properties/SVGPropertyTraits.h (288133 => 288134)


--- trunk/Source/WebCore/svg/properties/SVGPropertyTraits.h	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebCore/svg/properties/SVGPropertyTraits.h	2022-01-18 19:59:53 UTC (rev 288134)
@@ -45,10 +45,10 @@
 template<>
 struct SVGPropertyTraits<Color> {
     static Color initialValue() { return Color(); }
-    static Color fromString(const String& string) { return CSSParser::parseColor(string.stripWhiteSpace()); }
+    static Color fromString(const String& string) { return CSSParser::parseColorWithoutContext(string.stripWhiteSpace()); }
     static std::optional<Color> parse(const QualifiedName&, const String& string)
     {
-        Color color = CSSParser::parseColor(string.stripWhiteSpace());
+        Color color = CSSParser::parseColorWithoutContext(string.stripWhiteSpace());
         if (!color.isValid())
             return std::nullopt;
         return color;

Modified: trunk/Source/WebKit/ChangeLog (288133 => 288134)


--- trunk/Source/WebKit/ChangeLog	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebKit/ChangeLog	2022-01-18 19:59:53 UTC (rev 288134)
@@ -1,3 +1,17 @@
+2022-01-18  Sam Weinig  <wei...@apple.com>
+
+        Canvas functions that take colors as strings don't support all the syntax that CSS supports
+        https://bugs.webkit.org/show_bug.cgi?id=235269
+
+        Reviewed by Darin Adler.
+
+        Update for rename of CSSParser::parseColor() to CSSParser::parseColorWithoutContext().
+
+        * UIProcess/API/wpe/WebKitColor.cpp:
+        (webkit_color_parse):
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
+        (WKBundlePageSetComposition):
+
 2022-01-18  Alex Christensen  <achristen...@webkit.org>
 
         Copy com.apple.WebKit.adattributiond.sb into place with other iOS sandbox profiles

Modified: trunk/Source/WebKit/UIProcess/API/wpe/WebKitColor.cpp (288133 => 288134)


--- trunk/Source/WebKit/UIProcess/API/wpe/WebKitColor.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebKit/UIProcess/API/wpe/WebKitColor.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -108,7 +108,7 @@
     g_return_val_if_fail(color, FALSE);
     g_return_val_if_fail(colorString, FALSE);
 
-    auto webCoreColor = WebCore::CSSParser::parseColor({ colorString });
+    auto webCoreColor = WebCore::CSSParser::parseColorWithoutContext({ colorString });
     if (!webCoreColor.isValid())
         return FALSE;
 

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp (288133 => 288134)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp	2022-01-18 19:36:11 UTC (rev 288133)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp	2022-01-18 19:59:53 UTC (rev 288134)
@@ -626,7 +626,7 @@
             highlights.uncheckedAppend({
                 static_cast<unsigned>(startOffset),
                 static_cast<unsigned>(startOffset + static_cast<API::UInt64*>(dictionary->get("length"))->value()),
-                WebCore::CSSParser::parseColor(static_cast<API::String*>(dictionary->get("color"))->string())
+                WebCore::CSSParser::parseColorWithoutContext(static_cast<API::String*>(dictionary->get("color"))->string())
             });
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to