Repository navigation
fix(web): guard parseBrowserLanguages against invalid locale tags - #190651
auto-submit[bot] merged 6 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the locale parsing logic in EnginePlatformDispatcher to handle invalid locale tags gracefully. It wraps the DomLocale instantiation and parsing in a try-catch block to ignore tags rejected by Intl.Locale (such as en-US@posix on Linux). Additionally, it replaces the assertion that locales must not be empty with a fallback that returns a list containing _defaultLocale when no valid locales are found. There are no review comments, and I have no further feedback to provide.
e7853e6 to
5d29668
Compare
|
@navaronbracke added tests for en-US@posix via debugOverrideBrowserLanguages and all-invalid falling back to en_US. fixed commit email so CLA matches. 5d29668. |
|
@navaronbracke added en-US@posix / mixed-invalid / empty cases via debugOverrideBrowserLanguages. Should match what you asked for. |
5d29668 to
d4c10b8
Compare
mdebbar
left a comment
There was a problem hiding this comment.
Thanks for contributing this fix!
95f447b
Signed-off-by: Sankalp Thakur <[email protected]>
95f447b to
2cf28b7
Compare
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Fixes #190510
Engine bootstrap calls DomLocale(language) for every navigator.languages entry with no validation. A bad tag (e.g. en-US@posix from Chromium when LANG is unset) throws during field init and leaves a blank page.
Guard each tag with try/catch and skip invalid ones. If all fail, fall back to _defaultLocale (en-US).
Tests: en-US@posix, mixed invalid+valid, empty list via debugOverrideBrowserLanguages.
AI tools helped with drafting.