fix: respect system language preferences on Windows & macOS #23247

sorah · 2020-04-23T04:13:14Z

Description of Change

This PR aims to fix #18829. Please refer to the issue for more details.

Add implementations to set user's OS language preferences to Accept-Language and navigator.languages, on Windows and macOS.

This also improves the CJK fallback font selection on Windows. Chromium uses Accept-Language to determine fallback fonts on Windows, especially kanji/han characters in CJK.

Previously the full preferences set to OS was not given to Chromium. For instance, when user sets en-US, ja-JP to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language is given to Accept-Language on Electron.

This patch is based on the idea given at #15532 /cc @nitsakh

Considerations

fix or feat?

❓ No sure this a feat or a fix. Might be a fix, and backports would be appreciated as I'm thinking this is a bug for CJK speakers.

WinRT API

In this implementation, GetThreadPreferedUILanguages API is used on Windows. But, I noticed this API doesn't provide the full list of user preferences.
Only the system locale (i.e. used for logon screen) and the user locale (i.e. UI used while logged in) are returned.

~~So, how do you think about adding ELECTRON_ACCEPT_LANGUAGE environment variable to give the preferences correctly, in addition to this change?~~

✅ Resolved using WinRT utility in Chromium #23247 (comment)

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation is changed or added
PR title follows semantic commit guidelines
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

notes: Fixed an issue with navigator.languages and Accept-Language did not fully respect users' language preferences on Windows and macOS. This also improved fallback font selection for CJK texts on Windows.

welcome · 2020-04-23T04:13:16Z

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

fix: don't overwrite prevent_default if default wasn't prevented
feat: add app.isPackaged() method
docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

Follow the JavaScript, C++, and Python coding style.
Run npm run lint locally to catch formatting errors earlier.
Document any user-facing changes you've made following the documentation styleguide.
Include tests when adding/changing behavior.
Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

hanazuki · 2020-04-23T08:48:07Z

I have a sample application to retrieve Windows 10's Preferred Language setting: https://github.com/hanazuki/win-accept-languages-sample

This program makes use of GlobalizationPreferences.Languages from WinRT library, which is available since Windows 10 as with the Preferred Languages setting itself.
We can try this method first, and if not supported by the running OS, fallback to GetThreadPreferedUILanguages.

Note on compatibility for old Windows:

Windows 7 and earlier does not support WinRT; we have to load RoInitialize and RoGetActivationFactory functions at runtime (i.e. using LoadLibrary&GetProcAddress) instead of linking runtimeobject.lib.
- My sample does not do this. Chromium seems to have a utility for it.
Windows 8 does not have GlobalizationPreferences; we can detect this case by checking the result of RoGetActivationFactory.

I hope this will help.

sorah · 2020-04-23T19:50:31Z

Nice. I regret that I did not imagine that Chrome should have such utility as Chrome is utilizing modern Windows APIs...

sorah · 2020-04-24T00:24:22Z

Changed to utilize Globalization API when available. Now trying to fix failing CIs.

sorah · 2020-04-24T02:37:19Z

CI Passed. Lint failure seems not relevant to my changes..

sorah · 2020-04-24T02:47:34Z

Opened #23266 for lint failure.

sorah · 2020-04-25T15:18:43Z

rebased and only linux-ia32 is failing with flaky timeout. my change is ready.

zcbenz · 2020-04-27T06:17:26Z

shell/browser/api/electron_api_web_contents.cc

@@ -535,7 +535,7 @@ void WebContents::InitWithSessionAndOptions(
  managed_web_contents()->GetView()->SetDelegate(this);

  auto* prefs = web_contents()->GetMutableRendererPrefs();
-  prefs->accept_languages = g_browser_process->GetApplicationLocale();
+  SetAcceptLanguages(prefs);


Can you move the language preferences code into separate files and then reference it here? Like this:

#include "shell/common/lang_util.h" prefs->accept_languages = GetSystemLanguage();

It would make the code easier to maintain, and we might want to reuse the code in other places.

I wasn't sure where to put this kind of code. Extracted: d947ed9

zcbenz · 2020-04-28T00:26:08Z

Can you fix the lint waring?

$ node ./script/lint.js
shell/common/language_util_mac.mm:14:  Add #include <string> for string  [build/include_what_you_use] [4]
shell/common/language_util_mac.mm:14:  Add #include <vector> for vector<>  [build/include_what_you_use] [4]
Total errors found: 2

sorah · 2020-04-28T00:34:13Z

Fixed, rebased & squashed.

shell/browser/api/electron_api_web_contents.cc

sorah · 2020-04-28T01:55:17Z

nah, l10n_util::GetApplicationLocale() would block. changed to return empty vector (as getting one from a browser process can satisfy) at b8f4e09, but suggestions welcomed for possible Linux implementation.

https://app.circleci.com/pipelines/github/electron/electron/23740/workflows/27fc2fc4-3530-4576-979d-508f912d3172/jobs/533455/steps

[453:0428/013109.271509:FATAL:thread_restrictions.cc(78)] Check failed: !g_blocking_disallowed.Get().Get(). Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the ThreadPool, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking (see its documentation for best practices).

#0 0x55f86110bea9 base::debug::CollectStackTrace()
#1 0x55f861044643 base::debug::StackTrace::StackTrace()
#2 0x55f861058aaf logging::LogMessage::~LogMessage()
#3 0x55f86105932e logging::LogMessage::~LogMessage()
#4 0x55f8610dc235 base::internal::AssertBlockingAllowed()
#5 0x55f8610d65b9 base::ScopedBlockingCall::ScopedBlockingCall()
#6 0x55f861113658 base::CreateDirectoryAndGetError()
#7 0x55f862a00cf6 ui::PathProvider()
#8 0x55f861078c82 base::PathService::Get()
#9 0x55f861ed854e ui::ResourceBundle::GetLocaleFilePath()
#10 0x55f861ed84ca ui::ResourceBundle::LocaleDataPakExists()
#11 0x55f861eceadc l10n_util::CheckAndResolveLocale()
#12 0x55f861ecf58b l10n_util::GetApplicationLocaleInternalNonMac()
#13 0x55f861ecf706 l10n_util::GetApplicationLocale()
#14 0x55f85db4b1c4 electron::GetPreferredLanguages()
#15 0x55f85da6ad16 electron::api::WebContents::InitWithSessionAndOptions()

deepak1556

LGTM, nice change 👍

This commit fixes electron#18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on electron#15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

trop · 2020-05-04T20:39:11Z

@ckerr has manually backported this PR to "9-x-y", please check out #23405

This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

trop · 2020-05-04T21:21:12Z

@ckerr has manually backported this PR to "8-x-y", please check out #23407

This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Sorah Fukumori <her@sorah.jp> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

sorah · 2020-05-05T03:18:27Z

thank you for treating this patch as a fix! 🤗

* fix: respect system language preferences on Win/macOS (#23247) This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> * fix: remove unintentionally-committed code Some excess code was accidentally included in the last commit's cherry-pick. * chore: fix extraneous #include found by lint * fix: correct another manual backport error :P Co-authored-by: Sorah Fukumori <her@sorah.jp> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

trop · 2020-05-05T16:44:29Z

@ckerr has manually backported this PR to "7-2-x", please check out #23420

* fix: respect system language preferences on Win/macOS (#23247) This commit fixes #18829 Previously the full preferences set to OS was not given to Chromium. Also, this commit improves fallback font selection for CJK text. Chromium uses browser languages to determine fallback fonts on Windows, especially kanji/han characters in CJK. For instance, when user sets 'en-US, ja-JP' to Accept-Language, while Chromium chooses Japanese font for kanji text, but Electron chooses Chinese font. This is because only the first language was given to Accept-Language on Electron. This patch is based on #15532 Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net> * fix: don't shadow the `w` local variable * fixup! fix: respect system language preferences on Win/macOS (#23247) Co-authored-by: Sorah Fukumori <her@sorah.jp> Co-authored-by: Nitish Sakhawalkar <nitsakh@icloud.com> Co-authored-by: Kasumi Hanazuki <kasumi@rollingapple.net>

electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 23, 2020

sorah changed the title ~~fix: set navigator.languages from OS preferences~~ feat: respect language preferences on Windows & macOS Apr 23, 2020

sorah force-pushed the langs branch 3 times, most recently from 68ba39b to e5f23cb Compare April 23, 2020 04:20

sorah mentioned this pull request Apr 24, 2020

Choosing appropriate fallback font list by system locale (especially on Windows) #18829

Closed

3 tasks

sorah force-pushed the langs branch from 02fbf14 to 3d9114c Compare April 24, 2020 02:42

sorah mentioned this pull request Apr 24, 2020

style: use build/include_directory for NOLINT #23266

Merged

3 tasks

sorah force-pushed the langs branch from 3d9114c to c58beaf Compare April 24, 2020 03:43

electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 24, 2020

sorah force-pushed the langs branch 3 times, most recently from ebcc0e9 to 02de6fa Compare April 25, 2020 13:40

zcbenz reviewed Apr 27, 2020

View reviewed changes

sorah force-pushed the langs branch from 0d9d16b to 78d7de6 Compare April 28, 2020 00:33

sorah force-pushed the langs branch from 78d7de6 to 510a454 Compare April 28, 2020 00:45

deepak1556 reviewed Apr 28, 2020

View reviewed changes

shell/browser/api/electron_api_web_contents.cc Show resolved Hide resolved

deepak1556 approved these changes Apr 28, 2020

View reviewed changes

sorah force-pushed the langs branch from b8f4e09 to dfbfb3d Compare April 28, 2020 02:01

ckerr mentioned this pull request May 4, 2020

fix: respect system language preferences on Windows & macOS #23405

Merged

4 tasks

trop bot added in-flight/9-x-y and removed needs-manual-bp/9-x-y labels May 4, 2020

ckerr mentioned this pull request May 4, 2020

fix: respect system language preferences on Windows & macOS #23407

Merged

4 tasks

trop bot added in-flight/8-x-y and removed needs-manual-bp/8-x-y labels May 4, 2020

sorah deleted the langs branch May 5, 2020 03:17

trop bot added merged/8-x-y and removed in-flight/8-x-y labels May 5, 2020

trop bot added the in-flight/8-x-y label May 5, 2020

ckerr mentioned this pull request May 5, 2020

fix: respect system language preferences on Win/macOS #23420

Merged

4 tasks

trop bot added in-flight/7-2-x and removed needs-manual-bp/7-2-x labels May 5, 2020

ckerr added a commit that referenced this pull request May 6, 2020

fixup! fix: respect system language preferences on Win/macOS (#23247)

5f1fbb1

trop bot added merged/7-2-x and removed in-flight/7-2-x labels May 7, 2020

ckerr removed the in-flight/8-x-y label May 12, 2020

nowayhaze mentioned this pull request Dec 17, 2020

Chinese font not rendered with the correct font variation laurent22/joplin#3253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: respect system language preferences on Windows & macOS #23247

fix: respect system language preferences on Windows & macOS #23247

sorah commented Apr 23, 2020 •

edited by deepak1556

welcome bot commented Apr 23, 2020

hanazuki commented Apr 23, 2020 •

edited

sorah commented Apr 23, 2020 •

edited

sorah commented Apr 24, 2020

sorah commented Apr 24, 2020

sorah commented Apr 24, 2020

sorah commented Apr 25, 2020

zcbenz Apr 27, 2020

sorah Apr 28, 2020

zcbenz commented Apr 28, 2020

sorah commented Apr 28, 2020

sorah commented Apr 28, 2020 •

edited

deepak1556 left a comment

trop bot commented May 4, 2020

trop bot commented May 4, 2020

sorah commented May 5, 2020

trop bot commented May 5, 2020

fix: respect system language preferences on Windows & macOS #23247

fix: respect system language preferences on Windows & macOS #23247

Conversation

sorah commented Apr 23, 2020 • edited by deepak1556

Description of Change

Considerations

fix or feat?

WinRT API

Checklist

Release Notes

welcome bot commented Apr 23, 2020

hanazuki commented Apr 23, 2020 • edited

sorah commented Apr 23, 2020 • edited

sorah commented Apr 24, 2020

sorah commented Apr 24, 2020

sorah commented Apr 24, 2020

sorah commented Apr 25, 2020

zcbenz Apr 27, 2020

Choose a reason for hiding this comment

sorah Apr 28, 2020

Choose a reason for hiding this comment

zcbenz commented Apr 28, 2020

sorah commented Apr 28, 2020

sorah commented Apr 28, 2020 • edited

deepak1556 left a comment

Choose a reason for hiding this comment

trop bot commented May 4, 2020

trop bot commented May 4, 2020

sorah commented May 5, 2020

trop bot commented May 5, 2020

sorah commented Apr 23, 2020 •

edited by deepak1556

hanazuki commented Apr 23, 2020 •

edited

sorah commented Apr 23, 2020 •

edited

sorah commented Apr 28, 2020 •

edited