Adds semantics locale support for web #171196
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mdebbar
reviewed
Jun 26, 2025
Comment on lines
+5108
to
+5112
| if (localeForChildren != configProvider.effective.locale) { | ||
| configProvider.updateConfig((SemanticsConfiguration config) { | ||
| config.locale = localeForChildren; | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Should we add a comment here explaining why we are not always setting the locale?
I presume this is done to avoid excessive adding of lang attribute as described here:
Contributor
Author
There was a problem hiding this comment.
This line is to update the locale only if it is different.
As for this diagram, I am starting to think whether just set lang for every div will be easier.
Contributor
There was a problem hiding this comment.
Ah okay, I misunderstood this code.
Look at my other comment for a potentially easy way to achieve this on web.
| /// {@macro flutter.widgets.ProxyWidget.child} | ||
| final Widget? child; | ||
|
|
||
| /// Whether this is the main localizations with for that represent the app's |
Contributor
There was a problem hiding this comment.
Suggested change
| /// Whether this is the main localizations with for that represent the app's | |
| /// Whether this is the main localizations widget that represent the app's |
| // If this is not application level, we need to explicit mark the | ||
| // semantics subtree with the locale. | ||
| localeForSubtree: widget.isApplicationLevel ? null : widget.locale, | ||
| container: !widget.isApplicationLevel, |
Contributor
There was a problem hiding this comment.
Why not make it always a container?
Contributor
Author
There was a problem hiding this comment.
the reason this has to be a container if we set localeForSubtree is to prevent it merge up that end up including sibling renderobject to be under it.
for isApplicationLevel is true, it doesn't set localeForSubtree so no need to force it to container. I will have a follow up pr that adds a dart:ui API to set the application locale directly if isApplicationLevel is set to true
mdebbar
reviewed
Jun 27, 2025
Comment on lines
821
to
831
| void _updateLocale() { | ||
| final String locale = semanticsObject.locale?.toString() ?? ''; | ||
| if (locale.isEmpty) { | ||
| removeAttribute('lang'); | ||
| return; | ||
| } | ||
| setAttribute('lang', locale); | ||
| } |
Contributor
There was a problem hiding this comment.
Can we do something like this to avoid setting the lang attribute everywhere:
Suggested change
| void _updateLocale() { | |
| final String locale = semanticsObject.locale?.toString() ?? ''; | |
| if (locale.isEmpty) { | |
| removeAttribute('lang'); | |
| return; | |
| } | |
| setAttribute('lang', locale); | |
| } | |
| void _updateLocale() { | |
| final String locale = semanticsObject.locale?.toString() ?? ''; | |
| final isSameAsParent = semanticsObject.locale == semanticsObject.parent?.locale; | |
| if (locale.isEmpty || isSameAsParent) { | |
| removeAttribute('lang'); | |
| return; | |
| } | |
| setAttribute('lang', locale); | |
| } |
Comment on lines
+5108
to
+5112
| if (localeForChildren != configProvider.effective.locale) { | ||
| configProvider.updateConfig((SemanticsConfiguration config) { | ||
| config.locale = localeForChildren; | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Ah okay, I misunderstood this code.
Look at my other comment for a potentially easy way to achieve this on web.
mdebbar
approved these changes
Jun 30, 2025
Contributor
|
autosubmit label was removed for flutter/flutter/171196, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Contributor
Author
|
migrating internal code cl/778235479 Nothing functionally wrong, but internal code relies on implicit merging of semantics tree. Since this pr adds a container to the localization widget, it messes with the implicit merging of customer's app. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 5, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Jul 21, 2025
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> toward flutter#98948 This sets the ground work for framework and web still need to implement: 1. Android and iOS support 2. set application locale. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
toward #98948
This sets the ground work for framework and web
still need to implement:
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.