Changeset 225808 in webkit

Timestamp:

Dec 12, 2017, 2:51:02 PM (8 years ago)

Author:

mmaxfield@apple.com

Message:

REGRESSION (Safari 11): custom <font-face> tag crashes a page
​https://bugs.webkit.org/show_bug.cgi?id=177848

Reviewed by Darin Adler.

Source/WebCore:

We currently use the CSS property parsers to parse SVG's <font-face> element attributes. Instead,
we should be using the CSS descriptor parsers to parse these attributes. However, this is a
fairly involved task, so until I can finish that, this patch fixes the crash. The crash is simple;
the descriptors shouldn't accept the universal keywords ("initial", "inherit", etc.) and our
font-face machinery assumes this. So the fix is just detect these keywords and explicitly disallow
them.

Test: svg/text/font-style-keyword.html

• svg/SVGFontFaceElement.cpp:

(WebCore::SVGFontFaceElement::parseAttribute):

LayoutTests:

• svg/text/font-style-keyword-expected.txt: Added.
• svg/text/font-style-keyword.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/text/font-style-keyword-expected.txt (added)
• LayoutTests/svg/text/font-style-keyword.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSValue.h (modified) (1 diff)
• Source/WebCore/svg/SVGFontFaceElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-12-12  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (Safari 11): custom <font-face> tag crashes a page
+        https://bugs.webkit.org/show_bug.cgi?id=177848
+
+        Reviewed by Darin Adler.
+
+        * svg/text/font-style-keyword-expected.txt: Added.
+        * svg/text/font-style-keyword.html: Added.
+
 2017-12-12  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-12-12  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (Safari 11): custom <font-face> tag crashes a page
+        https://bugs.webkit.org/show_bug.cgi?id=177848
+
+        Reviewed by Darin Adler.
+
+        We currently use the CSS property parsers to parse SVG's <font-face> element attributes. Instead,
+        we should be using the CSS descriptor parsers to parse these attributes. However, this is a
+        fairly involved task, so until I can finish that, this patch fixes the crash. The crash is simple;
+        the descriptors shouldn't accept the universal keywords ("initial", "inherit", etc.) and our
+        font-face machinery assumes this. So the fix is just detect these keywords and explicitly disallow
+        them.
+
+        Test: svg/text/font-style-keyword.html
+
+        * svg/SVGFontFaceElement.cpp:
+        (WebCore::SVGFontFaceElement::parseAttribute):
+
 2017-12-12  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/css/CSSValue.h

@@ -92 +92 @@
     bool isUnsetValue() const { return m_classType == UnsetClass; }
     bool isRevertValue() const { return m_classType == RevertClass; }
+    bool isGlobalKeyword() const { return isInheritedValue() || isInitialValue() || isUnsetValue() || isRevertValue(); }
     bool treatAsInitialValue(CSSPropertyID) const;
     bool treatAsInheritedValue(CSSPropertyID) const;

trunk/Source/WebCore/svg/SVGFontFaceElement.cpp

@@ -64 +64 @@
 void SVGFontFaceElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {    
-    CSSPropertyID propId = cssPropertyIdForSVGAttributeName(name);
-    if (propId > 0) {
+    CSSPropertyID propertyId = cssPropertyIdForSVGAttributeName(name);
+    if (propertyId > 0) {
         // FIXME: Parse using the @font-face descriptor grammars, not the property grammars.
-        m_fontFaceRule->mutableProperties().setProperty(propId, value, false);
+        auto& properties = m_fontFaceRule->mutableProperties();
+        bool valueChanged = properties.setProperty(propertyId, value);
+
+        if (valueChanged) {
+            // The above parser is designed for the font-face properties, not descriptors, and the properties accept the global keywords, but descriptors don't.
+            // Rather than invasively modifying the parser for the properties to have a special mode, we can simply detect the error condition after-the-fact and
+            // avoid it explicitly.
+            if (auto parsedValue = properties.getPropertyCSSValue(propertyId)) {
+                if (parsedValue->isGlobalKeyword())
+                    properties.removeProperty(propertyId);
+            }
+        }
+
         rebuildFontFace();
         return;