- Revision
- 233447
- Author
- [email protected]
- Date
- 2018-07-02 17:15:20 -0700 (Mon, 02 Jul 2018)
Log Message
[Cocoa] Google Fonts doesn't work if the user has the requested font locally-installed
https://bugs.webkit.org/show_bug.cgi?id=187228
<rdar://problem/40967280>
Reviewed by Brent Fulgham.
Source/WebCore:
This is due to the local() items in the src: descriptor in the @font-family blocks.
This is because of a behavior difference between CSSFontFaceSource::load() and CSSFontFaceSource::font().
load() is supposed to set the status() to Success iff the font can be used, and then CSSFontFaceSource::font()
is supposed to return the font itself to use. load() works by constructing a dummy FontDescription and
performing a system lookup (to see if the local font really exists). However, this dummy FontDescription
doesn't set the ShouldAllowUserInstalledFonts flag. Then, in CSSFontFaceSource::font(), a similar lookup is
performed, except this one has the original FontDescription (with the correct value of the
ShouldAllowUserInstalledFonts flag set. Therefore, the two functions disagree about the state of the flag.
When the CSSFontFaceSource's status gets set to Success, that means "this is the font face source that
represents the @font-face block" but when CSSFontFaceSource::font() returns nullptr, that means "The font face
source can't be used for some reason" so we then continue searching down the font-family list (and render the
text in Helvetica or whatever comes next).
The solution is simple - just set the ShouldAllowUserInstalledFonts flag correctly in the dummy
FontDescription.
Test: fast/text/user-installed-fonts/local.html
* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::allowUserInstalledFonts const):
* css/CSSFontFace.h:
* css/CSSFontFaceSet.cpp:
(WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
* css/CSSFontFaceSource.cpp:
(WebCore::CSSFontFaceSource::load):
Tools:
The test only fails before the patch if the lookup for Helvetica2 is allowed to occur.
* WebKitTestRunner/mac/TestControllerMac.mm:
(WTR::allowedFontFamilySet):
LayoutTests:
* fast/text/user-installed-fonts/local-expected.html: Added.
* fast/text/user-installed-fonts/local.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (233446 => 233447)
--- trunk/LayoutTests/ChangeLog 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/LayoutTests/ChangeLog 2018-07-03 00:15:20 UTC (rev 233447)
@@ -1,3 +1,14 @@
+2018-07-02 Myles C. Maxfield <[email protected]>
+
+ [Cocoa] Google Fonts doesn't work if the user has the requested font locally-installed
+ https://bugs.webkit.org/show_bug.cgi?id=187228
+ <rdar://problem/40967280>
+
+ Reviewed by Brent Fulgham.
+
+ * fast/text/user-installed-fonts/local-expected.html: Added.
+ * fast/text/user-installed-fonts/local.html: Added.
+
2018-07-02 Wenson Hsieh <[email protected]>
[WK1] editing/spelling/markers.html is failing on recent builds of macOS Mojave
Added: trunk/LayoutTests/fast/text/user-installed-fonts/local-expected.html (0 => 233447)
--- trunk/LayoutTests/fast/text/user-installed-fonts/local-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/user-installed-fonts/local-expected.html 2018-07-03 00:15:20 UTC (rev 233447)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("../../../resources/Ahem.otf");
+}
+</style>
+</head>
+<body>
+This test makes sure that local() doesn't find a user-installed font, and that
+local() will be skipped if it lists a user-installed font.
+The test passes if you see some black boxes below (and no text below).
+<div style="font: 48px 'WebFont';">Hello, World</div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/user-installed-fonts/local.html (0 => 233447)
--- trunk/LayoutTests/fast/text/user-installed-fonts/local.html (rev 0)
+++ trunk/LayoutTests/fast/text/user-installed-fonts/local.html 2018-07-03 00:15:20 UTC (rev 233447)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+ testRunner.installFakeHelvetica("Helvetica2-400");
+if (window.internals)
+ internals.settings.setShouldAllowUserInstalledFonts(false);
+</script>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: local("Helvetica2"), url("../../../resources/Ahem.otf");
+}
+</style>
+</head>
+<body>
+This test makes sure that local() doesn't find a user-installed font, and that
+local() will be skipped if it lists a user-installed font.
+The test passes if you see some black boxes below (and no text below).
+<div style="font: 48px 'WebFont', 'AmericanTypewriter';">Hello, World</div>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (233446 => 233447)
--- trunk/LayoutTests/platform/ios/TestExpectations 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2018-07-03 00:15:20 UTC (rev 233447)
@@ -3278,6 +3278,7 @@
webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript.html [ ImageOnlyFailure ]
webkit.org/b/180062 fast/text/user-installed-fonts/shadow.html [ ImageOnlyFailure ]
webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript-family.html [ ImageOnlyFailure ]
+webkit.org/b/187228 fast/text/user-installed-fonts/local.html [ ImageOnlyFailure ]
webkit.org/b/181838 js/slow-stress/Int32Array-alloc-huge-long-lived.html [ Slow ]
Modified: trunk/LayoutTests/platform/mac/TestExpectations (233446 => 233447)
--- trunk/LayoutTests/platform/mac/TestExpectations 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2018-07-03 00:15:20 UTC (rev 233447)
@@ -1688,7 +1688,9 @@
webkit.org/b/180062 [ ElCapitan Sierra HighSierra ] fast/text/user-installed-fonts/disable.html [ ImageOnlyFailure ]
webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript.html [ ImageOnlyFailure ]
webkit.org/b/180062 fast/text/user-installed-fonts/shadow.html [ ImageOnlyFailure ]
+webkit.org/b/187228 [ ElCapitan Sierra HighSierra ] fast/text/user-installed-fonts/local.html [ ImageOnlyFailure ]
+
webkit.org/b/180560 accessibility/mac/html5-input-number.html [ Pass Failure ]
webkit.org/b/180675 accessibility/mac/search-field-cancel-button.html [ Pass Failure ]
Modified: trunk/Source/WebCore/ChangeLog (233446 => 233447)
--- trunk/Source/WebCore/ChangeLog 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Source/WebCore/ChangeLog 2018-07-03 00:15:20 UTC (rev 233447)
@@ -1,3 +1,39 @@
+2018-07-02 Myles C. Maxfield <[email protected]>
+
+ [Cocoa] Google Fonts doesn't work if the user has the requested font locally-installed
+ https://bugs.webkit.org/show_bug.cgi?id=187228
+ <rdar://problem/40967280>
+
+ Reviewed by Brent Fulgham.
+
+ This is due to the local() items in the src: descriptor in the @font-family blocks.
+
+ This is because of a behavior difference between CSSFontFaceSource::load() and CSSFontFaceSource::font().
+ load() is supposed to set the status() to Success iff the font can be used, and then CSSFontFaceSource::font()
+ is supposed to return the font itself to use. load() works by constructing a dummy FontDescription and
+ performing a system lookup (to see if the local font really exists). However, this dummy FontDescription
+ doesn't set the ShouldAllowUserInstalledFonts flag. Then, in CSSFontFaceSource::font(), a similar lookup is
+ performed, except this one has the original FontDescription (with the correct value of the
+ ShouldAllowUserInstalledFonts flag set. Therefore, the two functions disagree about the state of the flag.
+
+ When the CSSFontFaceSource's status gets set to Success, that means "this is the font face source that
+ represents the @font-face block" but when CSSFontFaceSource::font() returns nullptr, that means "The font face
+ source can't be used for some reason" so we then continue searching down the font-family list (and render the
+ text in Helvetica or whatever comes next).
+
+ The solution is simple - just set the ShouldAllowUserInstalledFonts flag correctly in the dummy
+ FontDescription.
+
+ Test: fast/text/user-installed-fonts/local.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::CSSFontFace::allowUserInstalledFonts const):
+ * css/CSSFontFace.h:
+ * css/CSSFontFaceSet.cpp:
+ (WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
+ * css/CSSFontFaceSource.cpp:
+ (WebCore::CSSFontFaceSource::load):
+
2018-06-29 Ryosuke Niwa <[email protected]>
Generate event and event target interface types directly instead of via macros
Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (233446 => 233447)
--- trunk/Source/WebCore/css/CSSFontFace.cpp 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp 2018-07-03 00:15:20 UTC (rev 233447)
@@ -582,6 +582,13 @@
ASSERT(!m_sourcesPopulated);
}
+AllowUserInstalledFonts CSSFontFace::allowUserInstalledFonts() const
+{
+ if (m_fontSelector && m_fontSelector->document())
+ return m_fontSelector->document()->settings().shouldAllowUserInstalledFonts() ? AllowUserInstalledFonts::Yes : AllowUserInstalledFonts::No;
+ return AllowUserInstalledFonts::Yes;
+}
+
static Settings::FontLoadTimingOverride fontLoadTimingOverride(CSSFontSelector* fontSelector)
{
auto overrideValue = Settings::FontLoadTimingOverride::None;
Modified: trunk/Source/WebCore/css/CSSFontFace.h (233446 => 233447)
--- trunk/Source/WebCore/css/CSSFontFace.h 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Source/WebCore/css/CSSFontFace.h 2018-07-03 00:15:20 UTC (rev 233447)
@@ -161,6 +161,8 @@
bool purgeable() const;
+ AllowUserInstalledFonts allowUserInstalledFonts() const;
+
void updateStyleIfNeeded();
#if ENABLE(SVG_FONTS)
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (233446 => 233447)
--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2018-07-03 00:15:20 UTC (rev 233447)
@@ -113,7 +113,7 @@
Vector<Ref<CSSFontFace>> faces;
for (auto item : capabilities) {
- Ref<CSSFontFace> face = CSSFontFace::create(nullptr, nullptr, nullptr, true);
+ Ref<CSSFontFace> face = CSSFontFace::create(m_owningFontSelector, nullptr, nullptr, true);
Ref<CSSValueList> familyList = CSSValueList::createCommaSeparated();
familyList->append(CSSValuePool::singleton().createFontFamilyValue(familyName));
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (233446 => 233447)
--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2018-07-03 00:15:20 UTC (rev 233447)
@@ -179,6 +179,7 @@
FontCascadeDescription fontDescription;
fontDescription.setOneFamily(m_familyNameOrURI);
fontDescription.setComputedSize(1);
+ fontDescription.setShouldAllowUserInstalledFonts(m_face.allowUserInstalledFonts());
success = FontCache::singleton().fontForFamily(fontDescription, m_familyNameOrURI, nullptr, nullptr, FontSelectionSpecifiedCapabilities(), true);
}
setStatus(success ? Status::Success : Status::Failure);
Modified: trunk/Tools/ChangeLog (233446 => 233447)
--- trunk/Tools/ChangeLog 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Tools/ChangeLog 2018-07-03 00:15:20 UTC (rev 233447)
@@ -1,3 +1,16 @@
+2018-07-02 Myles C. Maxfield <[email protected]>
+
+ [Cocoa] Google Fonts doesn't work if the user has the requested font locally-installed
+ https://bugs.webkit.org/show_bug.cgi?id=187228
+ <rdar://problem/40967280>
+
+ Reviewed by Brent Fulgham.
+
+ The test only fails before the patch if the lookup for Helvetica2 is allowed to occur.
+
+ * WebKitTestRunner/mac/TestControllerMac.mm:
+ (WTR::allowedFontFamilySet):
+
2018-07-02 Ryosuke Niwa <[email protected]>
run-benchmark should include the version number of MotionMark's results
Modified: trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm (233446 => 233447)
--- trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm 2018-07-02 23:45:22 UTC (rev 233446)
+++ trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm 2018-07-03 00:15:20 UTC (rev 233447)
@@ -212,6 +212,7 @@
@"Helvetica CY",
@"Helvetica Neue",
@"Helvetica",
+ @"Helvetica2",
@"Herculanum",
@"Hiragino Kaku Gothic Pro",
@"Hiragino Kaku Gothic ProN",
