This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Use Windows Display Language #43341
Merged
Merged
Use Windows Display Language #43341
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
cd2f7dd
Use Windows Display Language
yaakovschectman 59708af
Update unit test
yaakovschectman ad40c6d
Remove WindowsRegistry; now unused
yaakovschectman 4771e8e
Debug only
yaakovschectman a0b705c
More debug
yaakovschectman c1c5cb0
More debug
yaakovschectman aaa69e2
Try init to 0
yaakovschectman dc7d13a
Remove debug code
yaakovschectman f0211ef
Mock out API call
yaakovschectman 9df1103
Format
yaakovschectman a1f01ee
Update shell/platform/windows/windows_proc_table.h
yaakovschectman fc4cd50
PR comment
yaakovschectman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
GetThreadPreferredUILanguagesorGetPreferredUILanguagespreferable? Should we use one as a fallback for the other?Our partner team switch to
GetPreferredUILanguages, but comments indicate this might have been unintended. We should make sure we align to prevent inconsistencies. See: http://cl/544490185There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like they are using the same method as here, there. And re: your comment on
WindowsProcTable, this shouldn't be necessary. Looks like I missed a nuance of the parameters to the function that didn't make a difference locally but did on devicelabThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you're right, I got confused by their helper's name.
The current tests depend on the machines environment. For example, the tests would fail if somehow a machine was configured such that
GetThreadPreferredUILanguagesreturns no languages.In my mind, the ideal test would be an integration test that mocks
GetThreadPreferredUILanguagesand then checks that the Windows embedder sends the expectedFlutterLocaleto the embedder API at startup. Through our mocks we can guarantee that the machine's language configuration can't cause unexpected test flakes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure that:
a) This is a concern, e.g. that the environment can return no preferred languages? The test already in place always expected at least one valid language, and I do not know if a windows env can have zero preferred languages between the thread, process, user, and system. And,
b) Doing so will not obscure the purpose of this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a hypothetical scenario, yes. Another hypothetical scenario would be that
GetThreadPreferredUILanguagesfails and returnsfalse. These hypothetical scenarios would cause flakes. To reduce this likelihood, our tests should be deterministic and we should control their inputs.My only concern is using the real
GetThreadPreferredUILanguagesimplementation in the test. I'm flexible on the test itself, I'd be happy with your current test too ifGetThreadPreferredUILanguageswas mocked out :)