REGRESSION (r243637): Some web fonts fail to load on Google docs
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2019 21:18:04 +0000 (21:18 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2019 21:18:04 +0000 (21:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200106

Reviewed by Geoffrey Garen.

Prior to r243637, WebKit threw an exception if the font family name didn't parse as a CSS identifier in the setter
of `family` IDL attribute of FontFace interface because WebKit implemented the old spec faithfully unlike Chrome
and Firefox which basically treated it as a string and didn't throw any exception. [1]

To account for this browser behavior difference, Google docs implemented a workaround for Safari which is to wrap
some identifiers in font family names with quotation marks: `docs-Amatic SC` as `'docs-Amatic SC'` and
`docs-Playfair Display` as `docs-'Playfair Display'`. Unfortunately, this in turn causes the latest Safari to
not match these font face family with those that appear in stylesheet since we no longer parse it as a CSS identifier.

This patch adds a site specific quirk for Google docs to undo this workaround by stripping away single quotation marks.

[1] See https://github.com/w3c/csswg-drafts/issues/3776 for the context.

* css/FontFace.cpp:
(WebCore::FontFace::create):
(WebCore::FontFace::setFamily):
* css/FontFace.h:
* css/FontFace.idl:
* page/Quirks.cpp:
(WebCore::Quirks::shouldStripQuotationMarkInFontFaceSetFamily const):
* page/Quirks.h:

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

Source/WebCore/ChangeLog
Source/WebCore/css/FontFace.cpp
Source/WebCore/css/FontFace.h
Source/WebCore/css/FontFace.idl
Source/WebCore/page/Quirks.cpp
Source/WebCore/page/Quirks.h

index a54803d..83c54e3 100644 (file)
@@ -1,3 +1,32 @@
+2019-07-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r243637): Some web fonts fail to load on Google docs
+        https://bugs.webkit.org/show_bug.cgi?id=200106
+
+        Reviewed by Geoffrey Garen.
+
+        Prior to r243637, WebKit threw an exception if the font family name didn't parse as a CSS identifier in the setter
+        of `family` IDL attribute of FontFace interface because WebKit implemented the old spec faithfully unlike Chrome
+        and Firefox which basically treated it as a string and didn't throw any exception. [1]
+
+        To account for this browser behavior difference, Google docs implemented a workaround for Safari which is to wrap
+        some identifiers in font family names with quotation marks: `docs-Amatic SC` as `'docs-Amatic SC'` and
+        `docs-Playfair Display` as `docs-'Playfair Display'`. Unfortunately, this in turn causes the latest Safari to
+        not match these font face family with those that appear in stylesheet since we no longer parse it as a CSS identifier.
+
+        This patch adds a site specific quirk for Google docs to undo this workaround by stripping away single quotation marks.
+
+        [1] See https://github.com/w3c/csswg-drafts/issues/3776 for the context.
+
+        * css/FontFace.cpp:
+        (WebCore::FontFace::create):
+        (WebCore::FontFace::setFamily):
+        * css/FontFace.h:
+        * css/FontFace.idl:
+        * page/Quirks.cpp:
+        (WebCore::Quirks::shouldStripQuotationMarkInFontFaceSetFamily const):
+        * page/Quirks.h:
+
 2019-07-25  Dean Jackson  <dino@apple.com>
 
         Add helper for ignoring deprecated implementation warnings
index 5123b8b..212245d 100644 (file)
@@ -38,6 +38,7 @@
 #include "Document.h"
 #include "FontVariantBuilder.h"
 #include "JSFontFace.h"
+#include "Quirks.h"
 #include "StyleProperties.h"
 #include <JavaScriptCore/ArrayBuffer.h>
 #include <JavaScriptCore/ArrayBufferView.h>
@@ -59,7 +60,7 @@ ExceptionOr<Ref<FontFace>> FontFace::create(Document& document, const String& fa
 
     bool dataRequiresAsynchronousLoading = true;
 
-    auto setFamilyResult = result->setFamily(family);
+    auto setFamilyResult = result->setFamily(document, family);
     if (setFamilyResult.hasException())
         return setFamilyResult.releaseException();
 
@@ -148,15 +149,19 @@ RefPtr<CSSValue> FontFace::parseString(const String& string, CSSPropertyID prope
     return CSSParser::parseFontFaceDescriptor(propertyID, string, HTMLStandardMode);
 }
 
-ExceptionOr<void> FontFace::setFamily(const String& family)
+ExceptionOr<void> FontFace::setFamily(Document& document, const String& family)
 {
     if (family.isEmpty())
         return Exception { SyntaxError };
 
+    String familyNameToUse = family;
+    if (familyNameToUse.contains('\'') && document.quirks().shouldStripQuotationMarkInFontFaceSetFamily())
+        familyNameToUse = family.removeCharacters([](auto character) { return character == '\''; });
+
     // 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));
+    list->append(CSSValuePool::singleton().createFontFamilyValue(familyNameToUse));
     bool success = m_backing->setFamilies(list);
     if (!success)
         return Exception { SyntaxError };
index 3d9a0ec..f05a7ef 100644 (file)
@@ -55,7 +55,7 @@ public:
     static Ref<FontFace> create(CSSFontFace&);
     virtual ~FontFace();
 
-    ExceptionOr<void> setFamily(const String&);
+    ExceptionOr<void> setFamily(Document&, const String&);
     ExceptionOr<void> setStyle(const String&);
     ExceptionOr<void> setWeight(const String&);
     ExceptionOr<void> setStretch(const String&);
index 9af7214..2002d33 100644 (file)
@@ -47,7 +47,7 @@ dictionary FontFaceDescriptors {
     ConstructorMayThrowException,
     Constructor(DOMString family, (DOMString or BinaryData) source, optional FontFaceDescriptors descriptors)
 ] interface FontFace {
-    attribute DOMString family;
+    [SetterCallWith=Document] attribute DOMString family;
     attribute DOMString style;
     attribute DOMString weight;
     attribute DOMString stretch;
index 7937ef5..3d4374f 100644 (file)
@@ -137,6 +137,15 @@ bool Quirks::hasBrokenEncryptedMediaAPISupportQuirk() const
     return m_hasBrokenEncryptedMediaAPISupportQuirk.value();
 }
 
+bool Quirks::shouldStripQuotationMarkInFontFaceSetFamily() const
+{
+    if (!needsQuirks())
+        return false;
+
+    auto host = m_document->topDocument().url().host();
+    return equalLettersIgnoringASCIICase(host, "docs.google.com");
+}
+
 bool Quirks::isTouchBarUpdateSupressedForHiddenContentEditable() const
 {
 #if PLATFORM(MAC)
index a9face1..006a42e 100644 (file)
@@ -48,6 +48,7 @@ public:
     bool needsPerDocumentAutoplayBehavior() const;
     bool shouldAutoplayForArbitraryUserGesture() const;
     bool hasBrokenEncryptedMediaAPISupportQuirk() const;
+    bool shouldStripQuotationMarkInFontFaceSetFamily() const;
 #if ENABLE(TOUCH_EVENTS)
     bool shouldDispatchSimulatedMouseEvents() const;
     bool shouldDispatchedSimulatedMouseEventsAssumeDefaultPrevented(EventTarget*) const;