Changeset 248018 in webkit

Timestamp:

Jul 30, 2019, 2:48:27 PM (7 years ago)

Author:

mmaxfield@apple.com

Message:

REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
​https://bugs.webkit.org/show_bug.cgi?id=200065
<rdar://problem/50912757>

Reviewed by Simon Fraser.

Source/WebCore:

Before r241288, we were mapping Japanese sans-serif to Hiragino Kaku Gothic ProN, which
has a 300 weight and a 600 weight. However, we can't use that font because it's user-installed,
so in r241288 we switched to using Hiragino Sans, which has a 300 weight, a 600 weight, and an
800 weight. According to the CSS font selection algorithm, sites that request a weight of 700
would get the 800 weight instead of the 600 weight, which caused the text to look too heavy.
Therefore, the apparent visual change is from a weight change from 600 to 800.

In general, this is working as intended. However, text on Yahoo Japan looks too heavy in weight

800. Instead, this patch adds a quirk specific to Yahoo Japan that overwrites any font requests

to give them a weight of 600 instead of 700. This way, the lighter font will be used.

No new tests because quirks cannot be tested.

• css/CSSFontSelector.cpp:

(WebCore::resolveGenericFamily):
(WebCore::CSSFontSelector::fontRangesForFamily):

• page/Quirks.cpp:

(WebCore::Quirks::shouldLightenJapaneseBoldSansSerif const):

• page/Quirks.h:

Source/WTF:

• wtf/Platform.h:

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/Platform.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/css/CSSFontSelector.cpp (modified) (6 diffs)
• WebCore/page/Quirks.cpp (modified) (1 diff)
• WebCore/page/Quirks.h (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-07-30  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
+        https://bugs.webkit.org/show_bug.cgi?id=200065
+        <rdar://problem/50912757>
+
+        Reviewed by Simon Fraser.
+
+        * wtf/Platform.h:
+
 2019-07-30  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Source/WTF/wtf/Platform.h

@@ -1617 +1617 @@
 #define HAVE_DESIGN_SYSTEM_UI_FONTS 1
 #endif
+
+#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 60000) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000)
+#define USE_HIRAGINO_SANS_WORKAROUND 1
+#endif

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-30  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION(r241288): Text on Yahoo Japan mobile looks too bold
+        https://bugs.webkit.org/show_bug.cgi?id=200065
+        <rdar://problem/50912757>
+
+        Reviewed by Simon Fraser.
+
+        Before r241288, we were mapping Japanese sans-serif to Hiragino Kaku Gothic ProN, which
+        has a 300 weight and a 600 weight. However, we can't use that font because it's user-installed,
+        so in r241288 we switched to using Hiragino Sans, which has a 300 weight, a 600 weight, and an
+        800 weight. According to the CSS font selection algorithm, sites that request a weight of 700
+        would get the 800 weight instead of the 600 weight, which caused the text to look too heavy.
+        Therefore, the apparent visual change is from a weight change from 600 to 800.
+
+        In general, this is working as intended. However, text on Yahoo Japan looks too heavy in weight
+        800. Instead, this patch adds a quirk specific to Yahoo Japan that overwrites any font requests
+        to give them a weight of 600 instead of 700. This way, the lighter font will be used.
+
+        No new tests because quirks cannot be tested.
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::resolveGenericFamily):
+        (WebCore::CSSFontSelector::fontRangesForFamily):
+        * page/Quirks.cpp:
+        (WebCore::Quirks::shouldLightenJapaneseBoldSansSerif const):
+        * page/Quirks.h:
+
 2019-07-30  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Source/WebCore/css/CSSFontSelector.cpp

@@ -47 +47 @@
 #include "FrameLoader.h"
 #include "Logging.h"
+#include "Quirks.h"
 #include "ResourceLoadObserver.h"
 #include "RuntimeEnabledFeatures.h"
@@ -275 +276 @@
 }
 
-static AtomString resolveGenericFamily(Document* document, const FontDescription& fontDescription, const AtomString& familyName)
+static Optional<AtomString> resolveGenericFamily(Document* document, const FontDescription& fontDescription, const AtomString& familyName)
 {
     auto platformResult = FontDescription::platformResolveGenericFamily(fontDescription.script(), fontDescription.locale(), familyName);
@@ -282 +283 @@
 
     if (!document)
-        return familyName;
+        return WTF::nullopt;
 
     const Settings& settings = document->settings();
@@ -302 +303 @@
         return settings.standardFontFamily(script);
 
-    return familyName;
+    return WTF::nullopt;
 }
 
@@ -313 +314 @@
     bool resolveGenericFamilyFirst = familyName == standardFamily;
 
-    AtomString familyForLookup = resolveGenericFamilyFirst ? resolveGenericFamily(m_document.get(), fontDescription, familyName) : familyName;
-    auto* face = m_cssFontFaceSet->fontFace(fontDescription.fontSelectionRequest(), familyForLookup);
+    AtomString familyForLookup = familyName;
+    Optional<FontDescription> overrideFontDescription;
+    const FontDescription* fontDescriptionForLookup = &fontDescription;
+    auto resolveGenericFamily = [&]() {
+        if (auto genericFamilyOptional = WebCore::resolveGenericFamily(m_document.get(), fontDescription, familyName)) {
+            if (m_document && m_document->quirks().shouldLightenJapaneseBoldSansSerif() && familyForLookup == sansSerifFamily && fontDescription.weight() == boldWeightValue() && fontDescription.script() == USCRIPT_KATAKANA_OR_HIRAGANA) {
+                overrideFontDescription = fontDescription;
+                overrideFontDescription->setWeight(FontSelectionValue(600));
+                fontDescriptionForLookup = &*overrideFontDescription;
+            }
+            familyForLookup = *genericFamilyOptional;
+        }
+    };
+
+    if (resolveGenericFamilyFirst)
+        resolveGenericFamily();
+    auto* face = m_cssFontFaceSet->fontFace(fontDescriptionForLookup->fontSelectionRequest(), familyForLookup);
     if (face) {
         if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled()) {
@@ -320 +336 @@
                 ResourceLoadObserver::shared().logFontLoad(*m_document, familyForLookup.string(), true);
         }
-        return face->fontRanges(fontDescription);
-    }
+        return face->fontRanges(*fontDescriptionForLookup);
+    }
+
     if (!resolveGenericFamilyFirst)
-        familyForLookup = resolveGenericFamily(m_document.get(), fontDescription, familyName);
-    auto font = FontCache::singleton().fontForFamily(fontDescription, familyForLookup);
+        resolveGenericFamily();
+    auto font = FontCache::singleton().fontForFamily(*fontDescriptionForLookup, familyForLookup);
     if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled()) {
         if (m_document)

trunk/Source/WebCore/page/Quirks.cpp

@@ -386 +386 @@
 }
 
+bool Quirks::shouldLightenJapaneseBoldSansSerif() const
+{
+#if USE(HIRAGINO_SANS_WORKAROUND)
+    if (!needsQuirks())
+        return false;
+
+    // lang="ja" style="font: bold sans-serif;" content would naturally get HiraginoSans-W8 here, but that's visually
+    // too bold. Instead, we should pick HiraginoSans-W6 instead.
+    // FIXME: webkit.org/b/200047 Remove this quirk.
+    auto host = m_document->topDocument().url().host();
+    return equalLettersIgnoringASCIICase(host, "m.yahoo.co.jp");
+#else
+    return false;
+#endif
+}
+
 // FIXME(<rdar://problem/50394969>): Remove after desmos.com adopts inputmode="none".
 bool Quirks::needsInputModeNoneImplicitly(const HTMLElement& element) const

trunk/Source/WebCore/page/Quirks.h

@@ -58 +58 @@
     bool needsInputModeNoneImplicitly(const HTMLElement&) const;
     bool needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const;
+    bool shouldLightenJapaneseBoldSansSerif() const;
 
     WEBCORE_EXPORT bool shouldDispatchSyntheticMouseEventsWhenModifyingSelection() const;