Incorrect use of String::foldCase for font family names
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2019 14:39:10 +0000 (14:39 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2019 14:39:10 +0000 (14:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194895

Reviewed by Myles C. Maxfield.

* platform/graphics/FontCascadeDescription.cpp:
(WebCore::FontCascadeDescription::familiesEqualForTextAutoSizing): Use
familyNamesAreEqual instead of calling convertToASCIILowercase directly.
(WebCore::FontCascadeDescription::familyNamesAreEqual): Use AtomicString's
operator== when we want case sensitive family name comparisons. This is a special
case to accomodate CoreText, which uses "."-prefix names for internal fonts that
are treated case sensitively. (Ideally webpages would not use these fonts at all.)
(WebCore::FontCascadeDescription::familyNameHash): Use AtomicString's existingHash
when we want case sensitive family name hashing.
(WebCore::FontCascadeDescription::foldedFamilyName): Take a String instead of an
AtomicString so we can use this at an additional call site. Converting from an
AtomicString to a String if free and automatic at the existing call sites. Use
convertToASCIILowercase instead of foldCase for three reasons: 1) Other functions
here are folding only ASCII case by using ASCIICaseInsensitiveHash, and this one
must be consistent. 2) this is considerably faster, and 3) font family names don't
need arbitrary Unicode case folding, it's only A-Z that should be folded.
* platform/graphics/FontCascadeDescription.h: Take a String instead of AtomicString
in the foldedFamilyName function.

* platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::FontDatabase::collectionForFamily): Instead of calling foldCase, use
FontCascadeDescription::foldedFamilyName to correctly fold font family names.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontCascadeDescription.cpp
Source/WebCore/platform/graphics/FontCascadeDescription.h
Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp

index e751eca..4403f70 100644 (file)
@@ -1,3 +1,33 @@
+2019-02-20  Darin Adler  <darin@apple.com>
+
+        Incorrect use of String::foldCase for font family names
+        https://bugs.webkit.org/show_bug.cgi?id=194895
+
+        Reviewed by Myles C. Maxfield.
+
+        * platform/graphics/FontCascadeDescription.cpp:
+        (WebCore::FontCascadeDescription::familiesEqualForTextAutoSizing): Use
+        familyNamesAreEqual instead of calling convertToASCIILowercase directly.
+        (WebCore::FontCascadeDescription::familyNamesAreEqual): Use AtomicString's
+        operator== when we want case sensitive family name comparisons. This is a special
+        case to accomodate CoreText, which uses "."-prefix names for internal fonts that
+        are treated case sensitively. (Ideally webpages would not use these fonts at all.)
+        (WebCore::FontCascadeDescription::familyNameHash): Use AtomicString's existingHash
+        when we want case sensitive family name hashing.
+        (WebCore::FontCascadeDescription::foldedFamilyName): Take a String instead of an
+        AtomicString so we can use this at an additional call site. Converting from an
+        AtomicString to a String if free and automatic at the existing call sites. Use
+        convertToASCIILowercase instead of foldCase for three reasons: 1) Other functions
+        here are folding only ASCII case by using ASCIICaseInsensitiveHash, and this one
+        must be consistent. 2) this is considerably faster, and 3) font family names don't
+        need arbitrary Unicode case folding, it's only A-Z that should be folded.
+        * platform/graphics/FontCascadeDescription.h: Take a String instead of AtomicString
+        in the foldedFamilyName function.
+
+        * platform/graphics/cocoa/FontCacheCoreText.cpp:
+        (WebCore::FontDatabase::collectionForFamily): Instead of calling foldCase, use
+        FontCascadeDescription::foldedFamilyName to correctly fold font family names.
+
 2019-02-25  Charlie Turner  <cturner@igalia.com>
 
         [EME][GStreamer] Replace caps field loop with gst_structure_remove_fields
index f75547a..3fc96bf 100644 (file)
@@ -63,6 +63,7 @@ FontCascadeDescription::FontCascadeDescription()
 }
 
 #if !USE_PLATFORM_SYSTEM_FALLBACK_LIST
+
 unsigned FontCascadeDescription::effectiveFamilyCount() const
 {
     return familyCount();
@@ -72,6 +73,7 @@ FontFamilySpecification FontCascadeDescription::effectiveFamilyAt(unsigned i) co
 {
     return familyAt(i);
 }
+
 #endif
 
 FontSelectionValue FontCascadeDescription::lighterWeight(FontSelectionValue weight)
@@ -107,7 +109,7 @@ bool FontCascadeDescription::familiesEqualForTextAutoSizing(const FontCascadeDes
         return false;
 
     for (unsigned i = 0; i < thisFamilyCount; ++i) {
-        if (!equalIgnoringASCIICase(familyAt(i), other.familyAt(i)))
+        if (!familyNamesAreEqual(familyAt(i), other.familyAt(i)))
             return false;
     }
 
@@ -118,29 +120,29 @@ bool FontCascadeDescription::familiesEqualForTextAutoSizing(const FontCascadeDes
 
 bool FontCascadeDescription::familyNamesAreEqual(const AtomicString& family1, const AtomicString& family2)
 {
-    // FIXME: <rdar://problem/33594253> CoreText matches dot-prefixed font names case sensitively. We should
-    // always take the case insensitive patch once this radar is fixed.
+#if PLATFORM(COCOA)
     if (family1.startsWith('.'))
-        return StringHash::equal(family1.string(), family2.string());
+        return family1 == family2;
+#endif
     return ASCIICaseInsensitiveHash::equal(family1, family2);
 }
 
 unsigned FontCascadeDescription::familyNameHash(const AtomicString& family)
 {
-    // FIXME: <rdar://problem/33594253> CoreText matches dot-prefixed font names case sensitively. We should
-    // always take the case insensitive patch once this radar is fixed.
+#if PLATFORM(COCOA)
     if (family.startsWith('.'))
-        return StringHash::hash(family.string());
+        return family.existingHash();
+#endif
     return ASCIICaseInsensitiveHash::hash(family);
 }
 
-String FontCascadeDescription::foldedFamilyName(const AtomicString& family)
+String FontCascadeDescription::foldedFamilyName(const String& family)
 {
-    // FIXME: <rdar://problem/33594253> CoreText matches dot-prefixed font names case sensitively. We should
-    // always take the case insensitive patch once this radar is fixed.
+#if PLATFORM(COCOA)
     if (family.startsWith('.'))
-        return family.string();
-    return family.string().foldCase();
+        return family;
+#endif
+    return family.convertToASCIILowercase();
 }
 
 } // namespace WebCore
index 6fa8330..0d51c5a 100644 (file)
@@ -59,7 +59,7 @@ public:
 
     static bool familyNamesAreEqual(const AtomicString&, const AtomicString&);
     static unsigned familyNameHash(const AtomicString&);
-    static String foldedFamilyName(const AtomicString&);
+    static String foldedFamilyName(const String&);
 
     unsigned effectiveFamilyCount() const;
     FontFamilySpecification effectiveFamilyAt(unsigned) const;
index 5285284..5778125 100644 (file)
@@ -887,7 +887,7 @@ public:
 
     const InstalledFontFamily& collectionForFamily(const String& familyName)
     {
-        auto folded = familyName.foldCase();
+        auto folded = FontCascadeDescription::foldedFamilyName(familyName);
         {
             std::lock_guard<Lock> locker(m_familyNameToFontDescriptorsLock);
             auto it = m_familyNameToFontDescriptors.find(folded);