Repository navigation
[A11y] Stop sending the brightness that the app reports back to the app - #10071
Draft
victorsanni wants to merge 1 commit into
Draft
victorsanni wants to merge 1 commit into
victorsanni wants to merge 1 commit into
Conversation
The brightness dropdown wrote every change of its state back to the app, including the brightness that the app reported and the reset that ServiceExtensionManager makes when the main isolate exits. The dropdown now calls AccessibilityController.setBrightness, and the controller looks up the brightness state again whenever the main isolate changes.
11 tasks
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Part of #9893. Independent of #10065, which touches the same files.
The brightness dropdown on the Accessibility page wrote every change of its state back to the app, including changes that came from the app:
ServiceExtensionManagermakes when the main isolate exits, so the manager did not restore the override. The page then listened to a state that the manager no longer updates.viewMetricsOverridecall that removes the brightness override posts abrightnessOverrideevent, which the page wrote back as a new override.Now the dropdown calls
AccessibilityController.setBrightness, so only a selection reaches the app, and the controller looks up the brightness state again whenever the main isolate changes.AccessibilityController.brightnessis read-only.Behavior change: the dropdown changes when the app or
ServiceExtensionManagerreports the change, not when a value is picked. At a breakpoint, it changes once the app resumes.Caveat:
ServiceExtensionManagerstill keeps the brightness that the app reports as an enabled state, because the extension has more than two values, and sets it again when the extension is registered again. So while DevTools is connected, the app stays at the brightness it last reported and stops following the system theme, even if nobody chose one. This is left for a follow-up PR.Controller tests cover that the brightness the app reports is not written back, and that a hot restart restores the selection and the page follows the new isolate. The
devtools_testfakes can now replace the main isolate and reset the service extension states, asServiceExtensionManagerdoes. There is no release note, because the page is behind theaccessibilityexperiment flag.Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.