Attachment 346260 Details for Bug 187723 – Patch

[patch] Patch

bug-187723-20180731222914.patch (text/plain), 17.54 KB, created by Myles C. Maxfield on 2018-07-31 22:29:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2018-07-31 22:29:15 PDT

Size: 17.54 KB

patch

obsolete

>Subversion Revision: 234449
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index aa22ed8f7f5c91863a4467637d4386267468093f..c3b8a890aa1f3b68d5fc989d380dabfe8f3d9b91 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,48 @@
>+2018-07-31  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [Cocoa] Ask platform for generic font family mappings
>+        https://bugs.webkit.org/show_bug.cgi?id=187723
>+        <rdar://problem/41892438>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WebKit API allows setting the generic font families for the USCRIPT_COMMON script.
>+        When trying to style a character with a generic font family, we first look to see if
>+        we have a mapping for the particular script the character is rendered with, and if we
>+        don't find a match, we then check USCRIPT_COMMON.
>+
>+        In the Cocoa ports, the only way families get set for non-USCRIPT_COMMON scripts (aka
>+        the only scripts which won't use the API families) is in
>+        SettingsBase::initializeDefaultFontFamilies(). That function only sets the families
>+        for the CJK scripts.
>+
>+        Because these hardcoded names in SettingsBase::initializeDefaultFontFamilies() are
>+        incorrect, and because these scripts are the only ones which won't consult with the
>+        API families, these scripts are the ones that can use CTFontDescriptorCreateForCSSFamily().
>+        CTFontDescriptorCreateForCSSFamily() won't return any user-installed fonts, and will
>+        return better results than our hardcoded names.
>+
>+        Test: fast/text/jp-sans-serif.html
>+
>+        * css/CSSComputedStyleDeclaration.cpp:
>+        * css/CSSFontSelector.cpp:
>+        (WebCore::resolveGenericFamily):
>+        * css/parser/CSSPropertyParser.cpp:
>+        (WebCore::consumeFontFamily):
>+        * page/cocoa/SettingsBaseCocoa.mm:
>+        (WebCore::SettingsBase::initializeDefaultFontFamilies):
>+        * platform/graphics/FontDescription.cpp:
>+        (WebCore::FontDescription::platformResolveGenericFamily const):
>+        * platform/graphics/FontDescription.h:
>+        * platform/graphics/cocoa/FontDescriptionCocoa.cpp:
>+        (WebCore::SystemFontDatabase::serifFamily):
>+        (WebCore::SystemFontDatabase::sansSerifFamily):
>+        (WebCore::SystemFontDatabase::cursiveFamily):
>+        (WebCore::SystemFontDatabase::fantasyFamily):
>+        (WebCore::SystemFontDatabase::monospaceFamily):
>+        (WebCore::SystemFontDatabase::clear):
>+        (WebCore::FontDescription::platformResolveGenericFamily const):
>+
> 2018-07-31  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [WIN] Fix tests for text with initial advances
>diff --git a/Source/WebCore/PAL/ChangeLog b/Source/WebCore/PAL/ChangeLog
>index d7bbd969f070ee808326da3344a138addb9bed99..367256c1ad17fc9452e18849dddc448f866d1306 100644
>--- a/Source/WebCore/PAL/ChangeLog
>+++ b/Source/WebCore/PAL/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-07-31  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [Cocoa] Ask platform for generic font family mappings
>+        https://bugs.webkit.org/show_bug.cgi?id=187723
>+        <rdar://problem/41892438>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * pal/spi/cocoa/CoreTextSPI.h:
>+
> 2018-07-30  Sihui Liu  <sihui_liu@apple.com>
> 
>         Add support for fetching and remove type  _WKWebsiteDataTypeHSTSCache
>diff --git a/Source/WebCore/PAL/pal/spi/cocoa/CoreTextSPI.h b/Source/WebCore/PAL/pal/spi/cocoa/CoreTextSPI.h
>index 8cbb0d890a33de9b343da89313ea3980714669d2..96ee5bf8aca0ed9fdf812b9213b009fc412379db 100644
>--- a/Source/WebCore/PAL/pal/spi/cocoa/CoreTextSPI.h
>+++ b/Source/WebCore/PAL/pal/spi/cocoa/CoreTextSPI.h
>@@ -79,6 +79,13 @@ extern const CFStringRef kCTFontPostScriptNameAttribute;
> extern const CFStringRef kCTFontUserInstalledAttribute;
> extern const CFStringRef kCTFontFallbackOptionAttribute;
> 
>+extern const CFStringRef kCTFontCSSFamilySerif;
>+extern const CFStringRef kCTFontCSSFamilySansSerif;
>+extern const CFStringRef kCTFontCSSFamilyCursive;
>+extern const CFStringRef kCTFontCSSFamilyFantasy;
>+extern const CFStringRef kCTFontCSSFamilyMonospace;
>+extern const CFStringRef kCTFontCSSFamilySystemUI;
>+
> bool CTFontTransformGlyphs(CTFontRef, CGGlyph glyphs[], CGSize advances[], CFIndex count, CTFontTransformOptions);
> 
> CGSize CTRunGetInitialAdvance(CTRunRef);
>@@ -87,6 +94,7 @@ void CTRunGetBaseAdvancesAndOrigins(CTRunRef, CFRange, CGSize baseAdvances[], CG
> CTTypesetterRef CTTypesetterCreateWithUniCharProviderAndOptions(CTUniCharProviderCallback, CTUniCharDisposeCallback, void* refCon, CFDictionaryRef options);
> bool CTFontGetVerticalGlyphsForCharacters(CTFontRef, const UniChar characters[], CGGlyph glyphs[], CFIndex count);
> void CTFontGetUnsummedAdvancesForGlyphsAndStyle(CTFontRef, CTFontOrientation, CGFontRenderingStyle, const CGGlyph[], CGSize advances[], CFIndex count);
>+CTFontDescriptorRef CTFontDescriptorCreateForCSSFamily(CFStringRef cssFamily, CFStringRef language);
> 
> CTFontDescriptorRef CTFontDescriptorCreateForUIType(CTFontUIFontType, CGFloat size, CFStringRef language);
> CTFontDescriptorRef CTFontDescriptorCreateWithTextStyle(CFStringRef style, CFStringRef size, CFStringRef language);
>diff --git a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp
>index 6e54eabb770f1eb8153d17993a7dd96130dbd744..2e370908561778ac1fae6eadb1eea352f286f235 100644
>--- a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp
>+++ b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp
>@@ -1732,7 +1732,6 @@ bool ComputedStyleExtractor::useFixedFontDefaultSize()
>     return style->fontDescription().useFixedDefaultSize();
> }
> 
>-
> static CSSValueID identifierForFamily(const AtomicString& family)
> {
>     if (family == cursiveFamily)
>diff --git a/Source/WebCore/css/CSSFontSelector.cpp b/Source/WebCore/css/CSSFontSelector.cpp
>index 9ad88a2a4df62a851786c0f34a9541d0a3cffdcc..77b4179d5fceb457862f2e19de8f4ec8a1d0fe5f 100644
>--- a/Source/WebCore/css/CSSFontSelector.cpp
>+++ b/Source/WebCore/css/CSSFontSelector.cpp
>@@ -268,8 +268,12 @@ void CSSFontSelector::fontCacheInvalidated()
>     dispatchInvalidationCallbacks();
> }
> 
>-static const AtomicString& resolveGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
>+static AtomicString resolveGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
> {
>+    auto platformResult = fontDescription.platformResolveGenericFamily(familyName);
>+    if (!platformResult.isNull())
>+        return platformResult;
>+
>     if (!document)
>         return familyName;
> 
>diff --git a/Source/WebCore/css/parser/CSSPropertyParser.cpp b/Source/WebCore/css/parser/CSSPropertyParser.cpp
>index 61773879128dba61339d0ad19da0d0cb4d66e23f..87b9c25006165e82b0bbb873403c3b1f55058932 100644
>--- a/Source/WebCore/css/parser/CSSPropertyParser.cpp
>+++ b/Source/WebCore/css/parser/CSSPropertyParser.cpp
>@@ -1040,16 +1040,13 @@ static RefPtr<CSSValueList> consumeFontFamily(CSSParserTokenRange& range)
> {
>     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
>     do {
>-        RefPtr<CSSValue> parsedValue = consumeGenericFamily(range);
>-        if (parsedValue) {
>+        if (RefPtr<CSSValue> parsedValue = consumeGenericFamily(range))
>             list->append(parsedValue.releaseNonNull());
>-        } else {
>-            parsedValue = consumeFamilyName(range);
>-            if (parsedValue) {
>+        else {
>+            if (RefPtr<CSSValue> parsedValue = consumeFamilyName(range))
>                 list->append(parsedValue.releaseNonNull());
>-            } else {
>+            else
>                 return nullptr;
>-            }
>         }
>     } while (consumeCommaIncludingWhitespace(range));
>     return list;
>diff --git a/Source/WebCore/page/cocoa/SettingsBaseCocoa.mm b/Source/WebCore/page/cocoa/SettingsBaseCocoa.mm
>index fd7211ac12f8f4685bb3fde95e81565b9ee5a71a..18a442c3f99e3892ca2795054b2b0fecf3616bfa 100644
>--- a/Source/WebCore/page/cocoa/SettingsBaseCocoa.mm
>+++ b/Source/WebCore/page/cocoa/SettingsBaseCocoa.mm
>@@ -67,6 +67,9 @@ static bool osakaMonoIsInstalled()
> 
> void SettingsBase::initializeDefaultFontFamilies()
> {
>+    // FIXME: These CJK fonts should never be referenced because FontDescription::platformResolveGenericFamily() will avoid using them.
>+    // We should delete these lines here.
>+
>     setStandardFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);
>     setSerifFontFamily("Songti TC", USCRIPT_TRADITIONAL_HAN);
>     setFixedFontFamily(sansSerifTraditionalHanFontFamily(), USCRIPT_TRADITIONAL_HAN);
>@@ -99,6 +102,9 @@ void SettingsBase::initializeDefaultFontFamilies()
> 
> void SettingsBase::initializeDefaultFontFamilies()
> {
>+    // FIXME: These CJK fonts should never be referenced because FontDescription::platformResolveGenericFamily() will avoid using them.
>+    // We should delete these lines here.
>+
>     // There is no serif Chinese font in default iOS installation.
>     setStandardFontFamily(sansSerifTraditionalHanFontFamily(), USCRIPT_TRADITIONAL_HAN);
>     setSerifFontFamily(sansSerifTraditionalHanFontFamily(), USCRIPT_TRADITIONAL_HAN);
>@@ -112,9 +118,9 @@ void SettingsBase::initializeDefaultFontFamilies()
>     setSansSerifFontFamily(sansSerifSimplifiedHanFontFamily(), USCRIPT_SIMPLIFIED_HAN);
> 
>     setStandardFontFamily("Hiragino Mincho ProN", USCRIPT_KATAKANA_OR_HIRAGANA);
>-    setFixedFontFamily("Hiragino Kaku Gothic ProN", USCRIPT_KATAKANA_OR_HIRAGANA);
>+    setFixedFontFamily("Hiragino Sans", USCRIPT_KATAKANA_OR_HIRAGANA);
>     setSerifFontFamily("Hiragino Mincho ProN", USCRIPT_KATAKANA_OR_HIRAGANA);
>-    setSansSerifFontFamily("Hiragino Kaku Gothic ProN", USCRIPT_KATAKANA_OR_HIRAGANA);
>+    setSansSerifFontFamily("Hiragino Sans", USCRIPT_KATAKANA_OR_HIRAGANA);
> 
>     // There is no serif Korean font in default iOS installation.
>     setStandardFontFamily("Apple SD Gothic Neo", USCRIPT_HANGUL);
>diff --git a/Source/WebCore/platform/graphics/FontDescription.cpp b/Source/WebCore/platform/graphics/FontDescription.cpp
>index 130002d0f6e252428cbca5c7d21a07ecc98f59c5..4e87dcd73e8ba4867c2e1d455a1dacdc1ae76415 100644
>--- a/Source/WebCore/platform/graphics/FontDescription.cpp
>+++ b/Source/WebCore/platform/graphics/FontDescription.cpp
>@@ -113,6 +113,11 @@ FontFamilySpecification FontCascadeDescription::effectiveFamilyAt(unsigned i) co
> {
>     return familyAt(i);
> }
>+
>+AtomicString FontDescription::platformResolveGenericFamily(const AtomicString&) const
>+{
>+    return nullAtom();
>+}
> #endif
> 
> FontSelectionValue FontCascadeDescription::lighterWeight(FontSelectionValue weight)
>diff --git a/Source/WebCore/platform/graphics/FontDescription.h b/Source/WebCore/platform/graphics/FontDescription.h
>index 13fcdb3cf5c3fe35190ec3c1204d63f8ad4420c3..3e53842b637dac6b3a5af464ff795f3851d6c778 100644
>--- a/Source/WebCore/platform/graphics/FontDescription.h
>+++ b/Source/WebCore/platform/graphics/FontDescription.h
>@@ -151,6 +151,8 @@ public:
>     void setFontStyleAxis(FontStyleAxis axis) { m_fontStyleAxis = axis == FontStyleAxis::ital; }
>     void setShouldAllowUserInstalledFonts(AllowUserInstalledFonts shouldAllowUserInstalledFonts) { m_shouldAllowUserInstalledFonts = static_cast<unsigned>(shouldAllowUserInstalledFonts); }
> 
>+    AtomicString platformResolveGenericFamily(const AtomicString& familyName) const;
>+
>     static void invalidateCaches();
> 
> private:
>diff --git a/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp b/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp
>index 42afa48c55cec91bec29da1625b63cb1425d44f3..5ea66e91c31e6f6eb7f2406ee588d1534b78b53d 100644
>--- a/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp
>+++ b/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp
>@@ -140,9 +140,54 @@ public:
>         }).iterator->value;
>     }
> 
>+    AtomicString serifFamily(const AtomicString& locale)
>+    {
>+        return serifFamilies.ensure(locale, [&] {
>+            auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilySerif, locale.string().createCFString().get()));
>+            return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
>+        }).iterator->value;
>+    }
>+
>+    AtomicString sansSerifFamily(const AtomicString& locale)
>+    {
>+        return sansSeriferifFamilies.ensure(locale, [&] {
>+            auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilySansSerif, locale.string().createCFString().get()));
>+            return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
>+        }).iterator->value;
>+    }
>+
>+    AtomicString cursiveFamily(const AtomicString& locale)
>+    {
>+        return cursiveFamilies.ensure(locale, [&] {
>+            auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilyCursive, locale.string().createCFString().get()));
>+            return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
>+        }).iterator->value;
>+    }
>+
>+    AtomicString fantasyFamily(const AtomicString& locale)
>+    {
>+        return fantasyFamilies.ensure(locale, [&] {
>+            auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilyFantasy, locale.string().createCFString().get()));
>+            return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
>+        }).iterator->value;
>+    }
>+
>+    AtomicString monospaceFamily(const AtomicString& locale)
>+    {
>+        return monospaceFamilies.ensure(locale, [&] {
>+            auto descriptor = adoptCF(CTFontDescriptorCreateForCSSFamily(kCTFontCSSFamilyMonospace, locale.string().createCFString().get()));
>+            return adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(descriptor.get(), kCTFontFamilyNameAttribute))).get();
>+        }).iterator->value;
>+    }
>+
>     void clear()
>     {
>         m_systemFontCache.clear();
>+        serifFamilies.clear();
>+        sansSeriferifFamilies.clear();
>+        cursiveFamilies.clear();
>+        fantasyFamilies.clear();
>+        monospaceFamilies.clear();
>     }
> 
> private:
>@@ -193,6 +238,11 @@ private:
>     }
> 
>     HashMap<CoreTextCascadeListParameters, Vector<RetainPtr<CTFontDescriptorRef>>, CoreTextCascadeListParameters::CoreTextCascadeListParametersHash, SimpleClassHashTraits<CoreTextCascadeListParameters>> m_systemFontCache;
>+    HashMap<AtomicString, AtomicString> serifFamilies;
>+    HashMap<AtomicString, AtomicString> sansSeriferifFamilies;
>+    HashMap<AtomicString, AtomicString> cursiveFamilies;
>+    HashMap<AtomicString, AtomicString> fantasyFamilies;
>+    HashMap<AtomicString, AtomicString> monospaceFamilies;
> };
> 
> static inline bool isSystemFontString(const AtomicString& string)
>@@ -349,7 +399,31 @@ FontFamilySpecification FontCascadeDescription::effectiveFamilyAt(unsigned index
>     return nullAtom();
> }
> 
>+AtomicString FontDescription::platformResolveGenericFamily(const AtomicString& familyName) const
>+{
>+    UScriptCode script = this->script();
>+
>+    // FIXME: It is completely arbitrary that we do this for CJK scripts but no other scripts.
>+    // We have two signals: WebKit API can change the USCRIPT_GENERIC families,
>+    // and the platform has its own mappings.
>+    // We should figure out a more deliberate way to unify these two signals.
>+    if (script != USCRIPT_TRADITIONAL_HAN && script != USCRIPT_SIMPLIFIED_HAN && script != USCRIPT_KATAKANA_OR_HIRAGANA && script != USCRIPT_HANGUL)
>+        return nullAtom();
>+
>+    if (familyName == serifFamily)
>+        return SystemFontDatabase::singleton().serifFamily(locale());
>+    if (familyName == sansSerifFamily)
>+        return SystemFontDatabase::singleton().sansSerifFamily(locale());
>+    if (familyName == cursiveFamily)
>+        return SystemFontDatabase::singleton().cursiveFamily(locale());
>+    if (familyName == fantasyFamily)
>+        return SystemFontDatabase::singleton().fantasyFamily(locale());
>+    if (familyName == monospaceFamily)
>+        return SystemFontDatabase::singleton().monospaceFamily(locale());
>+
>+    return nullAtom();
> }
> 
>-#endif
>+}
> 
>+#endif
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f2356331a865753da2ffcbeec905673af95363af..8934334eb5c0908f6cdbb54f8ae2f1358fb97ea9 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-07-31  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [Cocoa] Ask platform for generic font family mappings
>+        https://bugs.webkit.org/show_bug.cgi?id=187723
>+        <rdar://problem/41892438>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/text/jp-sans-serif-expected-mismatch.html: Added.
>+        * fast/text/jp-sans-serif.html: Added.
>+
> 2018-07-31  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Tidy up a layout test introduced in r234436.
>diff --git a/LayoutTests/fast/text/jp-sans-serif-expected-mismatch.html b/LayoutTests/fast/text/jp-sans-serif-expected-mismatch.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e6a8a44cce9635f726def042d02966b046f10a2e
>--- /dev/null
>+++ b/LayoutTests/fast/text/jp-sans-serif-expected-mismatch.html
>@@ -0,0 +1,9 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+</head>
>+<body>
>+<div lang="jp" style="font: 48px serif;">ãããããªã·</div>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/text/jp-sans-serif.html b/LayoutTests/fast/text/jp-sans-serif.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..14eb9a044b66fb8c7547bd8aeb2692ed6c26bd13
>--- /dev/null
>+++ b/LayoutTests/fast/text/jp-sans-serif.html
>@@ -0,0 +1,9 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+</head>
>+<body>
>+<div lang="jp" style="font: 48px sans-serif;">ãããããªã·</div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 187723: 345137 | 345200 | 345277 | 345301 | 346259 | 346260 | 346264 | 360532 | 360536 | 360538 | 360539 | 360540 | 360610 | 360624 | 360628 | 360631 | 360633 | 360654 | 360664 | 360665 | 360671 | 360679 | 360887 | 360901 | 360905 | 360909 | 360911 | 360917 | 360924 | 360932 | 360935 | 360968 | 360982 | 360985 | 360987 | 361078 | 361580 | 361737 | 361738