From 97425d3cb1f30fc56784e72fcf330be685a7ce88 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp
Date: Tue, 30 Oct 2018 17:14:23 -0700
Subject: [PATCH 1/6] fix: match Chrome's font fallback behavior
Fixes #15481
---
BUILD.gn | 1 +
atom/browser/atom_browser_client.cc | 3 +
atom/browser/font_defaults.cc | 149 ++++++++++++++++++++++++++++
atom/browser/font_defaults.h | 18 ++++
electron_paks.gni | 2 +
filenames.gni | 2 +
6 files changed, 175 insertions(+)
create mode 100644 atom/browser/font_defaults.cc
create mode 100644 atom/browser/font_defaults.h
diff --git a/BUILD.gn b/BUILD.gn
index 9782fe4cdbd6d..1ed1a0e34f30a 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -208,6 +208,7 @@ static_library("electron_lib") {
"//base",
"//base:base_static",
"//base:i18n",
+ "//chrome/app/resources:platform_locale_settings",
"//chrome/common",
"//components/certificate_transparency",
"//components/net_log",
diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc
index 7f18f0750b93d..30774955141b2 100644
--- a/atom/browser/atom_browser_client.cc
+++ b/atom/browser/atom_browser_client.cc
@@ -21,6 +21,7 @@
#include "atom/browser/atom_resource_dispatcher_host_delegate.h"
#include "atom/browser/atom_speech_recognition_manager_delegate.h"
#include "atom/browser/child_web_contents_tracker.h"
+#include "atom/browser/font_defaults.h"
#include "atom/browser/io_thread.h"
#include "atom/browser/media/media_capture_devices_dispatcher.h"
#include "atom/browser/native_window.h"
@@ -301,6 +302,8 @@ void AtomBrowserClient::OverrideWebkitPrefs(content::RenderViewHost* host,
prefs->default_maximum_page_scale_factor = 1.f;
prefs->navigate_on_drag_drop = false;
+ SetFontDefaults(prefs);
+
// Custom preferences of guest page.
auto* web_contents = content::WebContents::FromRenderViewHost(host);
auto* web_preferences = WebContentsPreferences::From(web_contents);
diff --git a/atom/browser/font_defaults.cc b/atom/browser/font_defaults.cc
new file mode 100644
index 0000000000000..d6b6d4f6e78e0
--- /dev/null
+++ b/atom/browser/font_defaults.cc
@@ -0,0 +1,149 @@
+// Copyright (c) 2018 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/font_defaults.h"
+
+#include
+
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/grit/platform_locale_settings.h"
+#include "content/public/common/web_preferences.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace {
+
+struct FontDefault {
+ const char* pref_name;
+ int resource_id;
+};
+
+// Font pref defaults. The prefs that have defaults vary by platform, since not
+// all platforms have fonts for all scripts for all generic families.
+// TODO(falken): add proper defaults when possible for all
+// platforms/scripts/generic families.
+const FontDefault kFontDefaults[] = {
+ {prefs::kWebKitStandardFontFamily, IDS_STANDARD_FONT_FAMILY},
+ {prefs::kWebKitFixedFontFamily, IDS_FIXED_FONT_FAMILY},
+ {prefs::kWebKitSerifFontFamily, IDS_SERIF_FONT_FAMILY},
+ {prefs::kWebKitSansSerifFontFamily, IDS_SANS_SERIF_FONT_FAMILY},
+ {prefs::kWebKitCursiveFontFamily, IDS_CURSIVE_FONT_FAMILY},
+ {prefs::kWebKitFantasyFontFamily, IDS_FANTASY_FONT_FAMILY},
+ {prefs::kWebKitPictographFontFamily, IDS_PICTOGRAPH_FONT_FAMILY},
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX) || defined(OS_WIN)
+ {prefs::kWebKitStandardFontFamilyJapanese,
+ IDS_STANDARD_FONT_FAMILY_JAPANESE},
+ {prefs::kWebKitFixedFontFamilyJapanese, IDS_FIXED_FONT_FAMILY_JAPANESE},
+ {prefs::kWebKitSerifFontFamilyJapanese, IDS_SERIF_FONT_FAMILY_JAPANESE},
+ {prefs::kWebKitSansSerifFontFamilyJapanese,
+ IDS_SANS_SERIF_FONT_FAMILY_JAPANESE},
+ {prefs::kWebKitStandardFontFamilyKorean, IDS_STANDARD_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitSerifFontFamilyKorean, IDS_SERIF_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitSansSerifFontFamilyKorean,
+ IDS_SANS_SERIF_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitStandardFontFamilySimplifiedHan,
+ IDS_STANDARD_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitSerifFontFamilySimplifiedHan,
+ IDS_SERIF_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitSansSerifFontFamilySimplifiedHan,
+ IDS_SANS_SERIF_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitStandardFontFamilyTraditionalHan,
+ IDS_STANDARD_FONT_FAMILY_TRADITIONAL_HAN},
+ {prefs::kWebKitSerifFontFamilyTraditionalHan,
+ IDS_SERIF_FONT_FAMILY_TRADITIONAL_HAN},
+ {prefs::kWebKitSansSerifFontFamilyTraditionalHan,
+ IDS_SANS_SERIF_FONT_FAMILY_TRADITIONAL_HAN},
+#endif
+#if defined(OS_MACOSX) || defined(OS_WIN)
+ {prefs::kWebKitCursiveFontFamilySimplifiedHan,
+ IDS_CURSIVE_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitCursiveFontFamilyTraditionalHan,
+ IDS_CURSIVE_FONT_FAMILY_TRADITIONAL_HAN},
+#endif
+#if defined(OS_CHROMEOS)
+ {prefs::kWebKitStandardFontFamilyArabic, IDS_STANDARD_FONT_FAMILY_ARABIC},
+ {prefs::kWebKitSerifFontFamilyArabic, IDS_SERIF_FONT_FAMILY_ARABIC},
+ {prefs::kWebKitSansSerifFontFamilyArabic,
+ IDS_SANS_SERIF_FONT_FAMILY_ARABIC},
+ {prefs::kWebKitFixedFontFamilyKorean, IDS_FIXED_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitFixedFontFamilySimplifiedHan,
+ IDS_FIXED_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitFixedFontFamilyTraditionalHan,
+ IDS_FIXED_FONT_FAMILY_TRADITIONAL_HAN},
+#elif defined(OS_WIN)
+ {prefs::kWebKitFixedFontFamilyArabic, IDS_FIXED_FONT_FAMILY_ARABIC},
+ {prefs::kWebKitSansSerifFontFamilyArabic,
+ IDS_SANS_SERIF_FONT_FAMILY_ARABIC},
+ {prefs::kWebKitStandardFontFamilyCyrillic,
+ IDS_STANDARD_FONT_FAMILY_CYRILLIC},
+ {prefs::kWebKitFixedFontFamilyCyrillic, IDS_FIXED_FONT_FAMILY_CYRILLIC},
+ {prefs::kWebKitSerifFontFamilyCyrillic, IDS_SERIF_FONT_FAMILY_CYRILLIC},
+ {prefs::kWebKitSansSerifFontFamilyCyrillic,
+ IDS_SANS_SERIF_FONT_FAMILY_CYRILLIC},
+ {prefs::kWebKitStandardFontFamilyGreek, IDS_STANDARD_FONT_FAMILY_GREEK},
+ {prefs::kWebKitFixedFontFamilyGreek, IDS_FIXED_FONT_FAMILY_GREEK},
+ {prefs::kWebKitSerifFontFamilyGreek, IDS_SERIF_FONT_FAMILY_GREEK},
+ {prefs::kWebKitSansSerifFontFamilyGreek, IDS_SANS_SERIF_FONT_FAMILY_GREEK},
+ {prefs::kWebKitFixedFontFamilyKorean, IDS_FIXED_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitCursiveFontFamilyKorean, IDS_CURSIVE_FONT_FAMILY_KOREAN},
+ {prefs::kWebKitFixedFontFamilySimplifiedHan,
+ IDS_FIXED_FONT_FAMILY_SIMPLIFIED_HAN},
+ {prefs::kWebKitFixedFontFamilyTraditionalHan,
+ IDS_FIXED_FONT_FAMILY_TRADITIONAL_HAN},
+#endif
+};
+const size_t kFontDefaultsLength = arraysize(kFontDefaults);
+
+std::string GetDefaultFontForPref(const char* pref_name) {
+ for (size_t i = 0; i < kFontDefaultsLength; ++i) {
+ FontDefault pref = kFontDefaults[i];
+ if (strcmp(pref.pref_name, pref_name) == 0) {
+ return l10n_util::GetStringUTF8(pref.resource_id);
+ }
+ }
+ return std::string();
+}
+
+base::string16 FetchFont(const char* script, const char* map_name) {
+ std::string pref_name = base::StringPrintf("%s.%s", map_name, script);
+ std::string font = GetDefaultFontForPref(pref_name.c_str());
+ base::string16 font16 = base::UTF8ToUTF16(font);
+
+ return font16;
+}
+
+void FillFontFamilyMap(const char* map_name,
+ content::ScriptFontFamilyMap* map) {
+ for (size_t i = 0; i < prefs::kWebKitScriptsForFontFamilyMapsLength; ++i) {
+ const char* script = prefs::kWebKitScriptsForFontFamilyMaps[i];
+ base::string16 result = FetchFont(script, map_name);
+ if (!result.empty()) {
+ (*map)[script] = result;
+ }
+ }
+}
+
+} // namespace
+
+namespace atom {
+
+void SetFontDefaults(content::WebPreferences* prefs) {
+ FillFontFamilyMap(prefs::kWebKitStandardFontFamilyMap,
+ &prefs->standard_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitFixedFontFamilyMap,
+ &prefs->fixed_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitSerifFontFamilyMap,
+ &prefs->serif_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitSansSerifFontFamilyMap,
+ &prefs->sans_serif_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitCursiveFontFamilyMap,
+ &prefs->cursive_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitFantasyFontFamilyMap,
+ &prefs->fantasy_font_family_map);
+ FillFontFamilyMap(prefs::kWebKitPictographFontFamilyMap,
+ &prefs->pictograph_font_family_map);
+}
+
+} // namespace atom
diff --git a/atom/browser/font_defaults.h b/atom/browser/font_defaults.h
new file mode 100644
index 0000000000000..95ffe6b0194f2
--- /dev/null
+++ b/atom/browser/font_defaults.h
@@ -0,0 +1,18 @@
+// Copyright (c) 2018 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_FONT_DEFAULTS_H_
+#define ATOM_BROWSER_FONT_DEFAULTS_H_
+
+namespace content {
+struct WebPreferences;
+} // namespace content
+
+namespace atom {
+
+void SetFontDefaults(content::WebPreferences* prefs);
+
+} // namespace atom
+
+#endif // ATOM_BROWSER_FONT_DEFAULTS_H_
diff --git a/electron_paks.gni b/electron_paks.gni
index c1f577330099c..0db5201fdce29 100644
--- a/electron_paks.gni
+++ b/electron_paks.gni
@@ -143,12 +143,14 @@ template("electron_paks") {
}
source_patterns = [
+ "${root_gen_dir}/chrome/platform_locale_settings_",
"${root_gen_dir}/components/strings/components_strings_",
"${root_gen_dir}/content/app/strings/content_strings_",
"${root_gen_dir}/ui/strings/app_locale_settings_",
"${root_gen_dir}/ui/strings/ui_strings_",
]
deps = [
+ "//chrome/app/resources:platform_locale_settings",
"//components/strings:components_strings",
"//content/app/strings",
"//ui/strings:app_locale_settings",
diff --git a/filenames.gni b/filenames.gni
index 7ce785819ed94..e09be505abb74 100644
--- a/filenames.gni
+++ b/filenames.gni
@@ -120,6 +120,8 @@ filenames = {
"atom/app/uv_task_runner.cc",
"atom/app/uv_task_runner.h",
"atom/browser/api/atom_api_app.cc",
+ "atom/browser/font_defaults.cc",
+ "atom/browser/font_defaults.h",
"atom/browser/api/atom_api_app.h",
"atom/browser/api/atom_api_auto_updater.cc",
"atom/browser/api/atom_api_auto_updater.h",
From f92018d9bfc651410a75e3b1c5dd1388d58315d6 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp
Date: Fri, 2 Nov 2018 16:58:49 -0700
Subject: [PATCH 2/6] add a cache
---
atom/browser/font_defaults.cc | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/atom/browser/font_defaults.cc b/atom/browser/font_defaults.cc
index d6b6d4f6e78e0..fb8c496254158 100644
--- a/atom/browser/font_defaults.cc
+++ b/atom/browser/font_defaults.cc
@@ -5,6 +5,7 @@
#include "atom/browser/font_defaults.h"
#include
+#include
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
@@ -106,11 +107,30 @@ std::string GetDefaultFontForPref(const char* pref_name) {
return std::string();
}
+// Map from script to font.
+// Key comparison uses pointer equality.
+using ScriptFontMap = std::unordered_map;
+
+// Map from font family to ScriptFontMap.
+// Key comparison uses pointer equality.
+using FontFamilyMap = std::unordered_map;
+
+FontFamilyMap g_font_cache;
+
base::string16 FetchFont(const char* script, const char* map_name) {
+ FontFamilyMap::const_iterator it = g_font_cache.find(map_name);
+ if (it != g_font_cache.end()) {
+ ScriptFontMap::const_iterator it2 = it->second.find(script);
+ if (it2 != it->second.end())
+ return it2->second;
+ }
+
std::string pref_name = base::StringPrintf("%s.%s", map_name, script);
std::string font = GetDefaultFontForPref(pref_name.c_str());
base::string16 font16 = base::UTF8ToUTF16(font);
+ ScriptFontMap& map = g_font_cache[map_name];
+ map[script] = font16;
return font16;
}
From e19df881d77372d55dbd33dccfb5fa7e8a16e680 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp
Date: Mon, 5 Nov 2018 17:55:56 -0800
Subject: [PATCH 3/6] add test
---
spec/chromium-spec.js | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js
index 5cf0645699862..15276223ed3e1 100644
--- a/spec/chromium-spec.js
+++ b/spec/chromium-spec.js
@@ -10,6 +10,7 @@ const ChildProcess = require('child_process')
const { ipcRenderer, remote } = require('electron')
const { closeWindow } = require('./window-helpers')
const { resolveGetters } = require('./assert-helpers')
+const { emittedOnce } = require('./events-helpers')
const { app, BrowserWindow, ipcMain, protocol, session, webContents } = remote
const isCI = remote.getGlobal('isCi')
const features = process.atomBinding('features')
@@ -1312,3 +1313,43 @@ describe('chromium feature', () => {
})
})
})
+
+describe('font fallback', () => {
+ it('should fall back to non-PingFang SC font for sans-serif japanese script', async () => {
+ const w = new BrowserWindow({ show: false })
+ try {
+ const html = `
+
+
+
+
+
+ test 智史
+
+
+ `
+ const fallbackFontJa = {
+ 'win32': 'Meiryo',
+ 'darwin': 'Hiragino Kaku Gothic ProN',
+ 'linux': 'VL PGothic'
+ }[process.platform]
+ const loaded = emittedOnce(w.webContents, 'did-finish-load')
+ w.loadURL(`data:text/html,${html}`)
+ await loaded
+ w.webContents.debugger.attach()
+ const sendCommand = (...args) => new Promise((resolve, reject) => {
+ w.webContents.debugger.sendCommand(...args, (e, r) => {
+ if (e) { reject(e) } else { resolve(r) }
+ })
+ })
+ const { nodeId } = (await sendCommand('DOM.getDocument')).root.children[0]
+ await sendCommand('CSS.enable')
+ const { fonts } = await sendCommand('CSS.getPlatformFontsForNode', {nodeId})
+ expect(fonts).to.be.an('array')
+ expect(fonts).to.have.length(1)
+ expect(fonts[0].familyName).to.equal(fallbackFontJa)
+ } finally {
+ w.close()
+ }
+ })
+})
From 5c8187f5da9a31f4876427ea29a3e87c417f2687 Mon Sep 17 00:00:00 2001
From: Jeremy Apthorp
Date: Tue, 6 Nov 2018 13:30:32 -0800
Subject: [PATCH 4/6] another test
---
atom/browser/font_defaults.cc | 12 ++++++++
spec/chromium-spec.js | 56 ++++++++++++++++++++++-------------
2 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/atom/browser/font_defaults.cc b/atom/browser/font_defaults.cc
index fb8c496254158..85e57feb9d708 100644
--- a/atom/browser/font_defaults.cc
+++ b/atom/browser/font_defaults.cc
@@ -16,6 +16,14 @@
namespace {
+// The following list of font defaults was copied from
+// https://chromium.googlesource.com/chromium/src/+/69.0.3497.106/chrome/browser/ui/prefs/prefs_tab_helper.cc#152
+//
+// The only updates that should be made to this list are copying updates that
+// were made in Chromium.
+//
+// vvvvv DO NOT EDIT vvvvv
+
struct FontDefault {
const char* pref_name;
int resource_id;
@@ -97,6 +105,8 @@ const FontDefault kFontDefaults[] = {
};
const size_t kFontDefaultsLength = arraysize(kFontDefaults);
+// ^^^^^ DO NOT EDIT ^^^^^
+
std::string GetDefaultFontForPref(const char* pref_name) {
for (size_t i = 0; i < kFontDefaultsLength; ++i) {
FontDefault pref = kFontDefaults[i];
@@ -115,6 +125,8 @@ using ScriptFontMap = std::unordered_map;
// Key comparison uses pointer equality.
using FontFamilyMap = std::unordered_map;
+// A lookup table mapping (font-family, script) -> font-name
+// e.g. ("sans-serif", "Zyyy") -> "Arial"
FontFamilyMap g_font_cache;
base::string16 FetchFont(const char* script, const char* map_name) {
diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js
index 15276223ed3e1..195720d8fd33b 100644
--- a/spec/chromium-spec.js
+++ b/spec/chromium-spec.js
@@ -1315,24 +1315,9 @@ describe('chromium feature', () => {
})
describe('font fallback', () => {
- it('should fall back to non-PingFang SC font for sans-serif japanese script', async () => {
+ async function getRenderedFonts (html) {
const w = new BrowserWindow({ show: false })
try {
- const html = `
-
-
-
-
-
- test 智史
-
-
- `
- const fallbackFontJa = {
- 'win32': 'Meiryo',
- 'darwin': 'Hiragino Kaku Gothic ProN',
- 'linux': 'VL PGothic'
- }[process.platform]
const loaded = emittedOnce(w.webContents, 'did-finish-load')
w.loadURL(`data:text/html,${html}`)
await loaded
@@ -1344,12 +1329,43 @@ describe('font fallback', () => {
})
const { nodeId } = (await sendCommand('DOM.getDocument')).root.children[0]
await sendCommand('CSS.enable')
- const { fonts } = await sendCommand('CSS.getPlatformFontsForNode', {nodeId})
- expect(fonts).to.be.an('array')
- expect(fonts).to.have.length(1)
- expect(fonts[0].familyName).to.equal(fallbackFontJa)
+ const { fonts } = await sendCommand('CSS.getPlatformFontsForNode', { nodeId })
+ return fonts
} finally {
w.close()
}
+ }
+
+ it('should use Helvetica for sans-serif on mac, and Arial on windows and linux', async () => {
+ const html = `test `
+ const fonts = await getRenderedFonts(html)
+ expect(fonts).to.be.an('array')
+ expect(fonts).to.have.length(1)
+ expect(fonts[0].familyName).to.equal({
+ 'win32': 'Arial',
+ 'darwin': 'Helvetica',
+ 'linux': 'Arial'
+ }[process.platform])
+ })
+
+ it('should fall back to non-PingFang SC font for sans-serif japanese script', async () => {
+ if (process.platform === 'linux') {
+ return this.skip()
+ }
+ const html = `
+
+