Skip to content

Commit 039ebd9

Browse files
cdumezcdumez
committed
fast/text/international/system-language/navigator-language/navigator-language-fr.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=240104 <rdar://problem/92765233> Reviewed by Per Arne Vollan and Myles Maxfield. The test was flaky and navigator.language would sometimes return "fr" and sometimes "fr-FR". The reason for this is that Cocoa ports used 2 separate mechanisms to override the system language: 1. The override languages were used to set the AppleLanguages NSUserDefaults, causing APIs such as CFLocaleCopyPreferredLanguages() to return the overriden languages. 2. During process initialization we would also call WTF::overrideUserPreferredLanguages() which would add the override languages to an override Vector in WTF. The test was setting the override language to "fr". When method 2 would succeed, navigator.language would return "fr", from preferredLanguagesOverride(). However, Internals::resetToConsistentState() would reset WTF::preferredLanguagesOverride() shortly after the test starts running. As a result, the override Vector in WTF would often end up being empty and we would end up calling CFLocaleCopyPreferredLanguages(). However, CFLocaleCopyPreferredLanguages() return "fr-FR", which is equivalent but not exactly the same. To address the issue, I made the following changes: a. Use a single method for overriding languages for Cocoa ports. We are now using the AppleLanguages user default exclusively and not the WTF::preferredLanguagesOverride() Vector. b. We now call Internals::resetToConsistentState() only after running the test in WebKitTestRunner, not at the beginning of the test. This is consistent to what we were already doing in DumpRenderTree. Because Internals::resetToConsistentState() resets the languages, we don't want it to run at the beginning of the test. This is because some tests specify their languages in their header and WKTR ends up setting those languages via TestOptions, before actually running the test. We don't want those to get cleared. * Tools/WebKitTestRunner/TestController.cpp: (WTR::TestController::resetStateToConsistentValues): Only ask the injected bundle to reset after running the test and not before. This is consistent with what DumpRenderTree was already doing. The injected bundle would call Internals::resetToConsistentState() when receiving this reset message. * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm: (WebKit::setAppleLanguagesPreference): Move the logic to set the AppleLanguages user default from the WebKit2 layer to WTF, inside LanguageCocoa.mm and use it inside WTF::overrideUserPreferredLanguages(). * Source/WTF/wtf/Language.cpp: (WTF::overrideUserPreferredLanguages): * Source/WTF/wtf/cocoa/LanguageCocoa.mm: (WTF::overrideUserPreferredLanguages): Stop using the WTF::preferredLanguagesOverride() Vector on Cocoa and set the AppleLanguages user default instead. WebKit2 was using both AppleLanguages and this Vector, which would give inconsistent results, especially when resetting the Vector but not the user default. Using the user default is also more realistic then the fake override Vector as we end up calling into the usual CF APIs to retrieve the languages. * Source/WebCore/platform/graphics/FontDescription.cpp: (WebCore::computeSpecializedChineseLocale): * Source/WebCore/platform/graphics/FontGenericFamilies.cpp: (WebCore::computeUserPrefersSimplified): Add some FIXME comments. These functions do locale matching but use minimized locales which may cause the matching to fail. This is causing fast/text/international/generic-font-family-language-traditional.html to fail. We didn't notice before because our test infrastructure storing language overrides in a vector in WTF, which doesn't get minimized, unlike locales we get from the system. * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::userPreferredLanguages const): Stop minimizing the locales returned by internals.userPreferredLanguages(). It used to not matter because locales set by the test would be set in the WTF::overrideUserPreferredLanguages() Vector and locales from that vector would get returned un-minimized, even when requesting minimized locales. However, now that we are no longer using this vector and using the regular code path instead, locales would get minimized and this would cause a test to fail. The test was checking that the values returned by internals.userPreferredLanguages() are exactly the same as the ones set via internals.setUserPreferredLanguages(). * LayoutTests/fast/harness/user-preferred-language.html: Tweak test to use call internals.setUserPreferredLanguages() with proper locales instead of using non-locales. This used to not matter because we were storing them in the WTF::overrideUserPreferredLanguages() Vector and internals.userPreferredLanguages() would return then as-is from the vector. However, now that we actually set the AppleLanguages user default and actually call the CF APIs to retrieve the locales, having properly formatted locales is required. * LayoutTests/platform/gtk/fast/text/international/system-language/navigator-language/navigator-language-en-expected.txt: Added. We used to clear the language override shortly after starting the test so the test was actually getting the system language instead of "en" which is the override that the test sets. * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/mac/TestExpectations: Mark fast/text/international/generic-font-family-language-traditional.html as failing since language minimization is causing it to fail. This is not a regression from this patch. This is an issue in shipping code that is now exposed because our test infrastructure for locale overriding is now closer to real life. Canonical link: https://commits.webkit.org/250473@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294079 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8a92050 commit 039ebd9

File tree

12 files changed

+53
-35
lines changed

12 files changed

+53
-35
lines changed

‎JSTests/stress/intl-default-locale.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ shouldBe(Intl.getCanonicalLocales(new Intl.DateTimeFormat().resolvedOptions().lo
1313
shouldBe(Intl.getCanonicalLocales(new Intl.NumberFormat().resolvedOptions().locale)[0], new Intl.NumberFormat().resolvedOptions().locale);
1414
shouldBe(Intl.getCanonicalLocales(new Intl.Collator().resolvedOptions().locale)[0], new Intl.NumberFormat().resolvedOptions().locale);
1515

16-
// Any language name with less than two characters is considered invalid, so we use "a" here.
17-
// "i-klingon" is grandfathered, and is canonicalized "tlh".
18-
// It should not be part of any available locale sets, so we know it came from here.
19-
$vm.setUserPreferredLanguages([ "a", "*", "en_US.utf8", "i-klingon", "en-US" ]);
20-
shouldBe(new Intl.DateTimeFormat().resolvedOptions().locale, 'tlh');
21-
shouldBe(new Intl.NumberFormat().resolvedOptions().locale, 'tlh');
22-
shouldBe(new Intl.Collator().resolvedOptions().locale, 'tlh');
16+
$vm.setUserPreferredLanguages([ "fr-FR" ]);
17+
shouldBe(new Intl.DateTimeFormat().resolvedOptions().locale, 'fr-FR');
18+
shouldBe(new Intl.NumberFormat().resolvedOptions().locale, 'fr-FR');
19+
shouldBe(new Intl.Collator().resolvedOptions().locale, 'fr-FR');

‎LayoutTests/fast/harness/user-preferred-language.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919

2020
test('internals.userPreferredLanguages returns a non-empty array', languages.length);
2121

22-
languages.unshift("first-language");
23-
languages.push("last-language");
22+
languages.unshift("fr-LU");
23+
languages.push("de-LU");
2424
internals.setUserPreferredLanguages(languages);
2525

2626
var newLanguages = internals.userPreferredLanguages();

‎LayoutTests/platform/gtk/fast/text/international/system-language/navigator-language/navigator-language-en-expected.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
FAIL navigator.language should be en-US. Was en.
2+
PASS successfullyParsed is true
3+
Some tests failed.
4+
5+
TEST COMPLETE
6+

‎LayoutTests/platform/ios/TestExpectations

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3584,6 +3584,8 @@ webkit.org/b/239564 fast/forms/select-list-box-with-height.html [ Failure ]
35843584
webkit.org/b/239564 fast/forms/control-restrict-line-height.html [ Failure ]
35853585
webkit.org/b/239564 tables/mozilla/bugs/bug2479-3.html [ Failure ]
35863586

3587+
webkit.org/b/240268 fast/text/international/generic-font-family-language-traditional.html [ ImageOnlyFailure ]
3588+
35873589
webkit.org/b/239625 imported/w3c/web-platform-tests/css/css-color/opacity-overlapping-letters.html [ ImageOnlyFailure ]
35883590

35893591
webkit.org/b/239567 tables/mozilla/bugs/bug26178.html [ Pass Failure ]
@@ -3601,8 +3603,6 @@ webkit.org/b/237552 imported/w3c/web-platform-tests/html/browsers/history/the-lo
36013603

36023604
webkit.org/b/240081 webaudio/AudioBuffer/huge-buffer.html [ Pass Slow ]
36033605

3604-
webkit.org/b/240104 fast/text/international/system-language/navigator-language/navigator-language-fr.html [ Pass Failure ]
3605-
36063606
webkit.org/b/240123 imported/w3c/web-platform-tests/webrtc/protocol/rtp-clockrate.html [ Pass Failure ]
36073607

36083608
webkit.org/b/239568 imported/w3c/web-platform-tests/content-security-policy/inheritance/blob-url-in-main-window-self-navigate-inherits.sub.html [ Pass Failure ]

‎LayoutTests/platform/mac/TestExpectations

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,6 +2266,8 @@ webkit.org/b/226826 [ Monterey ] http/tests/ssl/applepay/ApplePayButton.html [ F
22662266

22672267
webkit.org/b/235660 [ Debug ] fast/replaced/encrypted-pdf-as-object-and-embed.html [ Skip ]
22682268

2269+
webkit.org/b/240268 fast/text/international/generic-font-family-language-traditional.html [ ImageOnlyFailure ]
2270+
22692271
# <model> tests involving the ready promise can only work on Monterey and up
22702272
[ BigSur ] model-element/model-element-ready.html [ Skip ]
22712273

‎Source/WTF/wtf/Language.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,6 @@ Vector<String> userPreferredLanguagesOverride()
113113
return preferredLanguagesOverride();
114114
}
115115

116-
void overrideUserPreferredLanguages(const Vector<String>& override)
117-
{
118-
LOG_WITH_STREAM(Language, stream << "Languages are being overridden to: " << override);
119-
{
120-
Locker locker { preferredLanguagesOverrideLock };
121-
preferredLanguagesOverride() = override;
122-
}
123-
languageDidChange();
124-
}
125-
126116
Vector<String> userPreferredLanguages(ShouldMinimizeLanguages shouldMinimizeLanguages)
127117
{
128118
{
@@ -146,6 +136,16 @@ Vector<String> userPreferredLanguages(ShouldMinimizeLanguages shouldMinimizeLang
146136

147137
#if !PLATFORM(COCOA)
148138

139+
void overrideUserPreferredLanguages(const Vector<String>& override)
140+
{
141+
LOG_WITH_STREAM(Language, stream << "Languages are being overridden to: " << override);
142+
{
143+
Locker locker { preferredLanguagesOverrideLock };
144+
preferredLanguagesOverride() = override;
145+
}
146+
languageDidChange();
147+
}
148+
149149
static String canonicalLanguageIdentifier(const String& languageCode)
150150
{
151151
String lowercaseLanguageCode = languageCode.convertToASCIILowercase();

‎Source/WTF/wtf/cocoa/LanguageCocoa.mm

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#import <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
3333
#import <wtf/cocoa/VectorCocoa.h>
3434
#import <wtf/spi/cocoa/NSLocaleSPI.h>
35+
#import <wtf/text/TextStream.h>
3536
#import <wtf/text/WTFString.h>
3637

3738
namespace WTF {
@@ -73,4 +74,14 @@ bool canMinimizeLanguages()
7374
ALLOW_NEW_API_WITHOUT_GUARDS_END
7475
}
7576

77+
void overrideUserPreferredLanguages(const Vector<String>& override)
78+
{
79+
LOG_WITH_STREAM(Language, stream << "Languages are being overridden to: " << override);
80+
NSDictionary *existingArguments = [[NSUserDefaults standardUserDefaults] volatileDomainForName:NSArgumentDomain];
81+
auto newArguments = adoptNS([existingArguments mutableCopy]);
82+
[newArguments setValue:createNSArray(override).get() forKey:@"AppleLanguages"];
83+
[[NSUserDefaults standardUserDefaults] setVolatileDomain:newArguments.get() forName:NSArgumentDomain];
84+
languageDidChange();
85+
}
86+
7687
}

‎Source/WebCore/platform/graphics/FontDescription.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ FontDescription::FontDescription()
7070

7171
static AtomString computeSpecializedChineseLocale()
7272
{
73+
// FIXME: This is not passing ShouldMinimizeLanguages::No and then getting minimized languages,
74+
// which may cause the matching below to fail.
7375
for (auto& language : userPreferredLanguages()) {
7476
if (startsWithLettersIgnoringASCIICase(language, "zh-"_s))
7577
return AtomString { language };

‎Source/WebCore/platform/graphics/FontGenericFamilies.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ static bool setGenericFontFamilyForScript(ScriptFontFamilyMap& fontMap, const St
4646

4747
static inline bool computeUserPrefersSimplified()
4848
{
49-
const Vector<String>& preferredLanguages = userPreferredLanguages();
50-
for (auto& language : preferredLanguages) {
49+
// FIXME: This is not passing ShouldMinimizeLanguages::No and then getting minimized languages,
50+
// which may cause the matching below to fail.
51+
for (auto& language : userPreferredLanguages()) {
5152
if (equalLettersIgnoringASCIICase(language, "zh-tw"_s))
5253
return false;
5354
if (equalLettersIgnoringASCIICase(language, "zh-cn"_s))

‎Source/WebCore/testing/Internals.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2263,7 +2263,7 @@ void Internals::advanceToNextMisspelling()
22632263

22642264
Vector<String> Internals::userPreferredLanguages() const
22652265
{
2266-
return WTF::userPreferredLanguages();
2266+
return WTF::userPreferredLanguages(ShouldMinimizeLanguages::No);
22672267
}
22682268

22692269
void Internals::setUserPreferredLanguages(const Vector<String>& languages)

0 commit comments

Comments
 (0)