Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PlatformDispatcher.locale to return something meaningful when there are no locales. #22608

Merged
merged 2 commits into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -549,9 +549,10 @@ class PlatformDispatcher {
/// This is the first locale selected by the user and is the user's primary
/// locale (the locale the device UI is displayed in)
///
/// This is equivalent to `locales.first` and will provide an empty non-null
/// locale if the [locales] list has not been set or is empty.
Locale get locale => locales.first;
/// This is equivalent to `locales.first`, except that it will provide an
/// undefined (using the language tag "und") non-null locale if the [locales]
/// list has not been set or is empty.
Locale get locale => locales.isEmpty ? const Locale.fromSubtags() : locales.first;

/// The full system-reported supported locales of the device.
///
Expand Down Expand Up @@ -1318,6 +1319,10 @@ class Locale {
/// [region](https://github.com/unicode-org/cldr/blob/master/common/validity/region.xml) for
/// each of languageCode, scriptCode and countryCode respectively.
///
/// The [languageCode] subtag is optional. When there is no language subtag,
/// the parameter should be omitted or set to "und". When not supplied, the
/// [languageCode] defaults to "und", an undefined language code.
///
/// The [countryCode] subtag is optional. When there is no country subtag,
/// the parameter should be omitted or passed `null` instead of an empty-string.
///
Expand Down
6 changes: 5 additions & 1 deletion lib/web_ui/lib/src/engine/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
EngineSemanticsOwner.instance.updateSemantics(update);
}

/// This is equivalent to `locales.first`, except that it will provide an
/// undefined (using the language tag "und") non-null locale if the [locales]
/// list has not been set or is empty.
///
/// We use the first locale in the [locales] list instead of the browser's
/// built-in `navigator.language` because browsers do not agree on the
/// implementation.
Expand All @@ -554,7 +558,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
///
/// * https://developer.mozilla.org/en-US/docs/Web/API/NavigatorLanguage/languages,
/// which explains browser quirks in the implementation notes.
ui.Locale get locale => locales.first;
ui.Locale get locale => locales.isEmpty ? const ui.Locale.fromSubtags() : locales.first;

/// The full system-reported supported locales of the device.
///
Expand Down
1 change: 1 addition & 0 deletions lib/web_ui/test/engine/window_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ void testMain() {
// that the list is populated again).
EnginePlatformDispatcher.instance.debugResetLocales();
expect(window.locales, isEmpty);
expect(window.locale, equals(const ui.Locale.fromSubtags()));
expect(localeChangedCount, 0);
html.window.dispatchEvent(html.Event('languagechange'));
expect(window.locales, isNotEmpty);
Expand Down
13 changes: 13 additions & 0 deletions testing/dart/window_hooks_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,17 @@ void main() {
expectEquals(window.padding.bottom, 0.0);
expectEquals(window.systemGestureInsets.bottom, 44.0);
});

test('PlatformDispatcher.locale returns unknown locale when locales is set to empty list', () {
late Locale locale;
runZoned(() {
window.onLocaleChanged = () {
locale = PlatformDispatcher.instance.locale;
};
});

_updateLocales(<String>[]);
expectEquals(locale, const Locale.fromSubtags());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API docs say that we return a Locale with language tag 'und', but https://api.flutter.dev/flutter/package-intl_locale/Locale/fromSubtags.html doesn't seem to guarantee that. Maybe we should be checking as much here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could change the docs for fromSubtags...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did both.

expectEquals(locale.languageCode, 'und');
});
}