Skip to content
Permalink
Browse files
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
  • Loading branch information
cdumez committed May 11, 2022
1 parent 8a92050 commit 039ebd9db2a695fef5b67d38da258851ed81086c
@@ -13,10 +13,7 @@ shouldBe(Intl.getCanonicalLocales(new Intl.DateTimeFormat().resolvedOptions().lo
shouldBe(Intl.getCanonicalLocales(new Intl.NumberFormat().resolvedOptions().locale)[0], new Intl.NumberFormat().resolvedOptions().locale);
shouldBe(Intl.getCanonicalLocales(new Intl.Collator().resolvedOptions().locale)[0], new Intl.NumberFormat().resolvedOptions().locale);

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

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

languages.unshift("first-language");
languages.push("last-language");
languages.unshift("fr-LU");
languages.push("de-LU");
internals.setUserPreferredLanguages(languages);

var newLanguages = internals.userPreferredLanguages();
@@ -0,0 +1,6 @@
FAIL navigator.language should be en-US. Was en.
PASS successfullyParsed is true
Some tests failed.

TEST COMPLETE

@@ -3584,6 +3584,8 @@ webkit.org/b/239564 fast/forms/select-list-box-with-height.html [ Failure ]
webkit.org/b/239564 fast/forms/control-restrict-line-height.html [ Failure ]
webkit.org/b/239564 tables/mozilla/bugs/bug2479-3.html [ Failure ]

webkit.org/b/240268 fast/text/international/generic-font-family-language-traditional.html [ ImageOnlyFailure ]

webkit.org/b/239625 imported/w3c/web-platform-tests/css/css-color/opacity-overlapping-letters.html [ ImageOnlyFailure ]

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

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

webkit.org/b/240104 fast/text/international/system-language/navigator-language/navigator-language-fr.html [ Pass Failure ]

webkit.org/b/240123 imported/w3c/web-platform-tests/webrtc/protocol/rtp-clockrate.html [ Pass Failure ]

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 ]
@@ -2266,6 +2266,8 @@ webkit.org/b/226826 [ Monterey ] http/tests/ssl/applepay/ApplePayButton.html [ F

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

webkit.org/b/240268 fast/text/international/generic-font-family-language-traditional.html [ ImageOnlyFailure ]

# <model> tests involving the ready promise can only work on Monterey and up
[ BigSur ] model-element/model-element-ready.html [ Skip ]

@@ -113,16 +113,6 @@ Vector<String> userPreferredLanguagesOverride()
return preferredLanguagesOverride();
}

void overrideUserPreferredLanguages(const Vector<String>& override)
{
LOG_WITH_STREAM(Language, stream << "Languages are being overridden to: " << override);
{
Locker locker { preferredLanguagesOverrideLock };
preferredLanguagesOverride() = override;
}
languageDidChange();
}

Vector<String> userPreferredLanguages(ShouldMinimizeLanguages shouldMinimizeLanguages)
{
{
@@ -146,6 +136,16 @@ Vector<String> userPreferredLanguages(ShouldMinimizeLanguages shouldMinimizeLang

#if !PLATFORM(COCOA)

void overrideUserPreferredLanguages(const Vector<String>& override)
{
LOG_WITH_STREAM(Language, stream << "Languages are being overridden to: " << override);
{
Locker locker { preferredLanguagesOverrideLock };
preferredLanguagesOverride() = override;
}
languageDidChange();
}

static String canonicalLanguageIdentifier(const String& languageCode)
{
String lowercaseLanguageCode = languageCode.convertToASCIILowercase();
@@ -32,6 +32,7 @@
#import <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
#import <wtf/cocoa/VectorCocoa.h>
#import <wtf/spi/cocoa/NSLocaleSPI.h>
#import <wtf/text/TextStream.h>
#import <wtf/text/WTFString.h>

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

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

}
@@ -70,6 +70,8 @@ FontDescription::FontDescription()

static AtomString computeSpecializedChineseLocale()
{
// FIXME: This is not passing ShouldMinimizeLanguages::No and then getting minimized languages,
// which may cause the matching below to fail.
for (auto& language : userPreferredLanguages()) {
if (startsWithLettersIgnoringASCIICase(language, "zh-"_s))
return AtomString { language };
@@ -46,8 +46,9 @@ static bool setGenericFontFamilyForScript(ScriptFontFamilyMap& fontMap, const St

static inline bool computeUserPrefersSimplified()
{
const Vector<String>& preferredLanguages = userPreferredLanguages();
for (auto& language : preferredLanguages) {
// FIXME: This is not passing ShouldMinimizeLanguages::No and then getting minimized languages,
// which may cause the matching below to fail.
for (auto& language : userPreferredLanguages()) {
if (equalLettersIgnoringASCIICase(language, "zh-tw"_s))
return false;
if (equalLettersIgnoringASCIICase(language, "zh-cn"_s))
@@ -2263,7 +2263,7 @@ void Internals::advanceToNextMisspelling()

Vector<String> Internals::userPreferredLanguages() const
{
return WTF::userPreferredLanguages();
return WTF::userPreferredLanguages(ShouldMinimizeLanguages::No);
}

void Internals::setUserPreferredLanguages(const Vector<String>& languages)
@@ -33,6 +33,8 @@
#import <pal/spi/cf/CFUtilitiesSPI.h>
#import <pal/spi/cocoa/LaunchServicesSPI.h>
#import <sys/sysctl.h>
#import <wtf/BlockPtr.h>
#import <wtf/Language.h>
#import <wtf/OSObjectPtr.h>
#import <wtf/RetainPtr.h>
#import <wtf/spi/darwin/XPCSPI.h>
@@ -47,18 +49,14 @@ static void setAppleLanguagesPreference()

if (xpc_object_t languages = xpc_dictionary_get_value(bootstrap.get(), "OverrideLanguages")) {
@autoreleasepool {
NSDictionary *existingArguments = [[NSUserDefaults standardUserDefaults] volatileDomainForName:NSArgumentDomain];
auto newArguments = adoptNS([existingArguments mutableCopy]);
RetainPtr<NSMutableArray> newLanguages = adoptNS([[NSMutableArray alloc] init]);
xpc_array_apply(languages, ^(size_t index, xpc_object_t value) {
[newLanguages addObject:[NSString stringWithCString:xpc_string_get_string_ptr(value) encoding:NSUTF8StringEncoding]];
Vector<String> newLanguages;
xpc_array_apply(languages, makeBlockPtr([&newLanguages](size_t index, xpc_object_t value) {
newLanguages.append(String::fromUTF8(xpc_string_get_string_ptr(value)));
return true;
});

LOG_WITH_STREAM(Language, stream << "Bootstrap message contains OverrideLanguages: " << newLanguages.get());
}).get());

[newArguments setValue:newLanguages.get() forKey:@"AppleLanguages"];
[[NSUserDefaults standardUserDefaults] setVolatileDomain:newArguments.get() forName:NSArgumentDomain];
LOG_WITH_STREAM(Language, stream << "Bootstrap message contains OverrideLanguages: " << newLanguages);
overrideUserPreferredLanguages(newLanguages);
}
} else
LOG(Language, "Bootstrap message does not contain OverrideLanguages");
@@ -1028,7 +1028,8 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
if (!jscOptions.empty())
setValue(resetMessageBody, "JSCOptions", jscOptions.c_str());

WKPagePostMessageToInjectedBundle(TestController::singleton().mainWebView()->page(), toWK("Reset").get(), resetMessageBody.get());
if (resetStage == ResetStage::AfterTest)
WKPagePostMessageToInjectedBundle(TestController::singleton().mainWebView()->page(), toWK("Reset").get(), resetMessageBody.get());

WKContextSetShouldUseFontSmoothing(TestController::singleton().context(), false);
WKContextSetCacheModel(TestController::singleton().context(), kWKCacheModelDocumentBrowser);

0 comments on commit 039ebd9

Please sign in to comment.