FontFace constructor throws an exception when there is a name which starts with a...
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Mar 2019 02:26:47 +0000 (02:26 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Mar 2019 02:26:47 +0000 (02:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196232
<rdar://problem/49293978>

Reviewed by Ryosuke Niwa.

Source/WebCore:

We were technically following the spec, but Chrome and Firefox are both consistent and it was making a website break.
This is just a short-term fix until the underlying https://bugs.webkit.org/show_bug.cgi?id=196381 is fixed.

Test: fast/text/font-face-family.html

* css/FontFace.cpp:
(WebCore::FontFace::setFamily):

LayoutTests:

* fast/text/font-face-family-expected.txt: Added.
* fast/text/font-face-family.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/text/font-face-family-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/font-face-family.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/FontFace.cpp
Source/WebCore/css/parser/CSSPropertyParser.cpp

index f95eb2c..793c79c 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-28  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        FontFace constructor throws an exception when there is a name which starts with a number
+        https://bugs.webkit.org/show_bug.cgi?id=196232
+        <rdar://problem/49293978>
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/text/font-face-family-expected.txt: Added.
+        * fast/text/font-face-family.html: Added.
+
 2019-03-28  Ryosuke Niwa  <rniwa@webkit.org>
 
         getBoundingClientRect always returns empty rect on a collapsed range
diff --git a/LayoutTests/fast/text/font-face-family-expected.txt b/LayoutTests/fast/text/font-face-family-expected.txt
new file mode 100644 (file)
index 0000000..474c51a
--- /dev/null
@@ -0,0 +1,12 @@
+PASS new FontFace('a', 'url(garbage.otf)') is non-null.
+PASS new FontFace('4a', 'url(garbage.otf)') is non-null.
+PASS new FontFace('4"a', 'url(garbage.otf)') is non-null.
+PASS new FontFace('4\'a', 'url(garbage.otf)') is non-null.
+PASS new FontFace('4\'a"b', 'url(garbage.otf)') is non-null.
+PASS (new FontFace('a b', 'url(garbage.otf)')).family is "a b"
+PASS (new FontFace('a b, c', 'url(garbage.otf)')).family is "a b, c"
+PASS (new FontFace('ab,c', 'url(garbage.otf)')).family is "ab,c"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/text/font-face-family.html b/LayoutTests/fast/text/font-face-family.html
new file mode 100644 (file)
index 0000000..37b670f
--- /dev/null
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+shouldBeNonNull("new FontFace('a', 'url(garbage.otf)')");
+shouldBeNonNull("new FontFace('4a', 'url(garbage.otf)')");
+shouldBeNonNull("new FontFace('4\"a', 'url(garbage.otf)')");
+shouldBeNonNull("new FontFace('4\\'a', 'url(garbage.otf)')");
+shouldBeNonNull("new FontFace('4\\'a\"b', 'url(garbage.otf)')");
+shouldBeEqualToString("(new FontFace('a b', 'url(garbage.otf)')).family", "a b");
+shouldBeEqualToString("(new FontFace('a b, c', 'url(garbage.otf)')).family", "a b, c");
+shouldBeEqualToString("(new FontFace('ab,c', 'url(garbage.otf)')).family", "ab,c");
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index e816356..3c9341f 100644 (file)
@@ -1,3 +1,19 @@
+2019-03-28  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        FontFace constructor throws an exception when there is a name which starts with a number
+        https://bugs.webkit.org/show_bug.cgi?id=196232
+        <rdar://problem/49293978>
+
+        Reviewed by Ryosuke Niwa.
+
+        We were technically following the spec, but Chrome and Firefox are both consistent and it was making a website break.
+        This is just a short-term fix until the underlying https://bugs.webkit.org/show_bug.cgi?id=196381 is fixed.
+
+        Test: fast/text/font-face-family.html
+
+        * css/FontFace.cpp:
+        (WebCore::FontFace::setFamily):
+
 2019-03-28  Justin Fan  <justin_fan@apple.com>
 
         [Web GPU] Replace 'unsigned long' with 'unsigned' when implementing u32 variables
index 2cc6086..5123b8b 100644 (file)
@@ -153,9 +153,11 @@ ExceptionOr<void> FontFace::setFamily(const String& family)
     if (family.isEmpty())
         return Exception { SyntaxError };
 
-    bool success = false;
-    if (auto value = parseString(family, CSSPropertyFontFamily))
-        success = m_backing->setFamilies(*value);
+    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=196381 Don't use a list here.
+    // See consumeFontFamilyDescriptor() in CSSPropertyParser.cpp for why we're using it.
+    auto list = CSSValueList::createCommaSeparated();
+    list->append(CSSValuePool::singleton().createFontFamilyValue(family));
+    bool success = m_backing->setFamilies(list);
     if (!success)
         return Exception { SyntaxError };
     return { };
@@ -293,6 +295,21 @@ ExceptionOr<void> FontFace::setDisplay(const String& display)
 String FontFace::family() const
 {
     m_backing->updateStyleIfNeeded();
+
+    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=196381 This is only here because CSSFontFace erroneously uses a list of values instead of a single value.
+    // See consumeFontFamilyDescriptor() in CSSPropertyParser.cpp.
+    if (m_backing->families()->length() == 1) {
+        if (m_backing->families()->item(0)) {
+            auto& item = *m_backing->families()->item(0);
+            if (item.isPrimitiveValue()) {
+                auto& primitiveValue = downcast<CSSPrimitiveValue>(item);
+                if (primitiveValue.isFontFamily()) {
+                    auto& fontFamily = primitiveValue.fontFamily();
+                    return fontFamily.familyName;
+                }
+            }
+        }
+    }
     return m_backing->families()->cssText();
 }
 
index 23c430e..54ca31d 100644 (file)
@@ -1087,7 +1087,7 @@ static RefPtr<CSSValueList> consumeFontFamily(CSSParserTokenRange& range)
 
 static RefPtr<CSSValueList> consumeFontFamilyDescriptor(CSSParserTokenRange& range)
 {
-    // FIXME-NEWPARSER: For compatibility with the old parser, we have to make
+    // FIXME-NEWPARSER: https://bugs.webkit.org/show_bug.cgi?id=196381 For compatibility with the old parser, we have to make
     // a list here, even though the list always contains only a single family name.
     // Once the old parser is gone, we can delete this function, make the caller
     // use consumeFamilyName instead, and then patch the @font-face code to