Bug 200043 - REGRESSION(r185816): In the Hong Kong locale, navigator.language reports it's in the Taiwan locale
Summary: REGRESSION(r185816): In the Hong Kong locale, navigator.language reports it's...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 206134
Blocks:
  Show dependency treegraph
 
Reported: 2019-07-23 13:00 PDT by Myles C. Maxfield
Modified: 2020-02-06 14:35 PST (History)
16 users (show)

See Also:


Attachments
Patch (47.67 KB, patch)
2019-07-23 13:07 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (3.32 MB, application/zip)
2019-07-23 14:16 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.71 MB, application/zip)
2019-07-23 14:35 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews115 for mac-highsierra (3.11 MB, application/zip)
2019-07-23 15:18 PDT, EWS Watchlist
no flags Details
Patch (45.31 KB, patch)
2019-10-18 15:06 PDT, Myles C. Maxfield
rniwa: review+
Details | Formatted Diff | Diff
Patch for reviewing (45.92 KB, patch)
2019-10-18 17:13 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (55.74 KB, patch)
2019-12-18 15:31 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (57.30 KB, patch)
2019-12-18 16:08 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (57.78 KB, patch)
2020-01-08 18:01 PST, Myles C. Maxfield
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2019-07-23 13:00:59 PDT
In the Hong Kong locale, navigator.language reports it's in the Taiwan locale
Comment 1 Myles C. Maxfield 2019-07-23 13:07:38 PDT
Created attachment 374701 [details]
Patch
Comment 2 Alexey Proskuryakov 2019-07-23 13:16:16 PDT
Comment on attachment 374701 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374701&action=review

> Source/WTF/ChangeLog:14
> +        Nowadays, CFLocaleCopyPreferredLanguages() reports real BCP47 language codes, so we don't have to do any

Does it actually perform the normalization, or is it just that System Preferences writes BCP47 language codes in new OS versions? People who've been using macOS for a while may still have non-BCP47 preferred languages.

One way to test would be to override AppleLanguages.

> Source/WTF/wtf/cf/LanguageCF.cpp:-59
> -    // 3. This should probably match what is sent by the network layer as Accept-Language, but currently, that's implemented separately.

This one remains true.
Comment 3 Alexey Proskuryakov 2019-07-23 13:28:33 PDT
As this adds differentiation especially for languages that are less frequently used, some people may now be uniquely identified with low-tech methods like looking at navigator.userAgent and navigator.language.

I'm all for improving language support, especially when removing such ugly hacks, however we may want to consult with privacy experts.
Comment 4 EWS Watchlist 2019-07-23 14:16:27 PDT
Comment on attachment 374701 [details]
Patch

Attachment 374701 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12796133

New failing tests:
js/dom/navigator-language.html
Comment 5 EWS Watchlist 2019-07-23 14:16:28 PDT
Created attachment 374708 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 EWS Watchlist 2019-07-23 14:35:12 PDT
Comment on attachment 374701 [details]
Patch

Attachment 374701 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12796226

New failing tests:
js/dom/navigator-language.html
Comment 7 EWS Watchlist 2019-07-23 14:35:14 PDT
Created attachment 374711 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 8 EWS Watchlist 2019-07-23 15:18:44 PDT
Comment on attachment 374701 [details]
Patch

Attachment 374701 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12796336

New failing tests:
js/dom/navigator-language.html
Comment 9 EWS Watchlist 2019-07-23 15:18:46 PDT
Created attachment 374713 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 10 Myles C. Maxfield 2019-10-18 15:06:25 PDT
Created attachment 381336 [details]
Patch
Comment 11 Myles C. Maxfield 2019-10-18 15:07:07 PDT
<rdar://problem/44119496>
Comment 12 Ryosuke Niwa 2019-10-18 15:40:30 PDT
Comment on attachment 381336 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381336&action=review

> Source/WTF/ChangeLog:17
> +        Reviewed by NOBODY (OOPS!).

Nit: This line should appear before the long description.
Comment 13 Myles C. Maxfield 2019-10-18 17:12:12 PDT
Oh, I forgot to update a test result.
Comment 14 Myles C. Maxfield 2019-10-18 17:13:58 PDT
Created attachment 381351 [details]
Patch for reviewing
Comment 15 Myles C. Maxfield 2019-10-23 12:06:31 PDT
Comment on attachment 381336 [details]
Patch

We're going to solve this a different way, which protects privacy better.
Comment 16 Myles C. Maxfield 2019-12-18 15:31:38 PST
Created attachment 386021 [details]
Patch
Comment 17 Myles C. Maxfield 2019-12-18 16:08:57 PST
Created attachment 386027 [details]
Patch
Comment 18 Ryosuke Niwa 2019-12-18 19:42:39 PST
The test failure looks real:

--- /Volumes/Data/worker/macOS-High-Sierra-Release-WK1-Tests-EWS/build/layout-test-results/js/dom/navigator-language-expected.txt
+++ /Volumes/Data/worker/macOS-High-Sierra-Release-WK1-Tests-EWS/build/layout-test-results/js/dom/navigator-language-actual.txt
@@ -2,7 +2,7 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS language is 'en'
+FAIL language should be en-US. Was en-us.
 PASS successfullyParsed is true
 
 TEST COMPLETE
Comment 19 Myles C. Maxfield 2020-01-08 18:01:44 PST
Created attachment 387174 [details]
Patch
Comment 20 Alexey Proskuryakov 2020-01-08 18:12:08 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

Just a comment about something that caught my eye, didn’t attempt to review the rest of the patch.

> LayoutTests/fast/text/international/system-language/navigator-language/navigator-language-en-GB.html:4
> +<script src="../../../../../resources/js-test-pre.js"></script>

Can all of the tests use js-test.js instead of pre/post? That’s generally preferred due to simplicity.
Comment 21 Dean Jackson 2020-01-09 10:48:51 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

>> LayoutTests/fast/text/international/system-language/navigator-language/navigator-language-en-GB.html:4
>> +<script src="../../../../../resources/js-test-pre.js"></script>
> 
> Can all of the tests use js-test.js instead of pre/post? That’s generally preferred due to simplicity.

Or just write your own small .js file in this directory to expose shouldBeEqualToString and do the dump as text stuff.
Comment 22 Myles C. Maxfield 2020-01-10 19:05:37 PST
Committed r254389: <https://trac.webkit.org/changeset/254389>
Comment 23 Myles C. Maxfield 2020-01-10 20:58:19 PST
Committed r254391: <https://trac.webkit.org/changeset/254391>
Comment 24 WebKit Commit Bot 2020-01-11 17:16:58 PST
Re-opened since this is blocked by bug 206134
Comment 25 Myles C. Maxfield 2020-01-11 17:40:27 PST
This broke internal builds. Investigating now.
Comment 26 Simon Fraser (smfr) 2020-01-11 20:46:37 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

> Source/WTF/wtf/cocoa/LanguageCocoa.mm:36
> +    static NeverDestroyed<bool> result = [NSLocale respondsToSelector:@selector(minimizedLanguagesFromLanguages:)];

static bool should be enough.
Comment 27 Myles C. Maxfield 2020-01-11 22:46:26 PST
https://bugs.webkit.org/show_bug.cgi?id=206135 should fix the internal builds.
Comment 28 Myles C. Maxfield 2020-01-11 23:16:52 PST
Committed r254412: <https://trac.webkit.org/changeset/254412>
Comment 29 Myles C. Maxfield 2020-01-11 23:22:55 PST
Revision list:

r254389
r254391
r254411
r254412
Comment 30 Myles C. Maxfield 2020-01-11 23:44:27 PST
Committed r254413: <https://trac.webkit.org/changeset/254413>
Comment 31 Myles C. Maxfield 2020-01-11 23:44:44 PST
Revision list:

r254389
r254391
r254411
r254412
r254413
Comment 32 Myles C. Maxfield 2020-01-12 13:30:32 PST
Committed r254415: <https://trac.webkit.org/changeset/254415>
Comment 33 Myles C. Maxfield 2020-01-12 13:30:50 PST
Revision list:

r254389
r254391
r254411
r254412
r254413
r254415
Comment 34 Brent Fulgham 2020-01-23 14:48:17 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

> Source/WTF/wtf/spi/cocoa/NSLocaleSPI.h:30
> +#import <InternationalSupport/NSLocale+InternationalSupportExtensions.h>

Does this exist on all supported macOS build targets?
Comment 35 Chris Dumez 2020-01-30 10:55:13 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

> Source/WTF/wtf/spi/cocoa/NSLocaleSPI.h:35
> ++ (nonnull NSArray<NSString *> *)minimizedLanguagesFromLanguages:(nonnull NSArray<NSString *> *)languages;

Note that on my macOS SDK, even though I do have the <InternationalSupport/NSLocale+InternationalSupportExtensions.h> header, it does not declare this method (minimizedLanguagesFromLanguages). My iOS SDK does have it though. Does this fix really work on macOS? Is it supposed to be iOS-only.
Comment 36 Chris Dumez 2020-01-30 11:00:56 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

>> Source/WTF/wtf/cocoa/LanguageCocoa.mm:36
>> +    static NeverDestroyed<bool> result = [NSLocale respondsToSelector:@selector(minimizedLanguagesFromLanguages:)];
> 
> static bool should be enough.

I guess this would just return false on macOS since it would not be able to find this method.
Comment 37 Myles C. Maxfield 2020-02-06 14:35:51 PST
Comment on attachment 387174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387174&action=review

>> Source/WTF/wtf/spi/cocoa/NSLocaleSPI.h:35
>> ++ (nonnull NSArray<NSString *> *)minimizedLanguagesFromLanguages:(nonnull NSArray<NSString *> *)languages;
> 
> Note that on my macOS SDK, even though I do have the <InternationalSupport/NSLocale+InternationalSupportExtensions.h> header, it does not declare this method (minimizedLanguagesFromLanguages). My iOS SDK does have it though. Does this fix really work on macOS? Is it supposed to be iOS-only.

Your macOS SDK is probably too old.

+ (nonnull NSArray <NSString *> *)minimizedLanguagesFromLanguages:(nonnull NSArray <NSString *> *)languages API_AVAILABLE(macos(10.15.4), ios(13.4), watchos(6.4), tvos(13.4));