Issue metadata
Sign in to add a comment
|
Accessibility Manager should support multiple accessibility extensions |
||||||||||||||||||||||
Issue descriptionRight now a lot of stuff is hard-coded for ChromeVox, we should refactor it so that we can implement more accessibility features using component extensions where that makes sense.
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18f89cf0b120afff95e064f437140bdbfb33ef36 commit 18f89cf0b120afff95e064f437140bdbfb33ef36 Author: dmazzoni <dmazzoni@chromium.org> Date: Thu Aug 04 22:23:50 2016 Move ChromeVox loading out of ComponentLoader. First, it isn't actually necessary to load ChromeVox from ComponentLoader. This code is probably historical. For quite some time, AccessibilityManager automatically loads ChromeVox on startup if the flag is set (and automatically loads/unloads when the flag changes). We should load it from only one place. Tested manually during startup and when logging into a session, both work fine without this call. Second, I intend to refactor AccessibilityManager so that it can support multiple accessibility component extensions. Rather than a separate function in ComponentLoader to load each one, I think it'd make more sense for AccessibilityManager to just call ComponentLoader with the appropriate parameters like the extension id and path. BUG= 634233 Review-Url: https://codereview.chromium.org/2212853002 Cr-Commit-Position: refs/heads/master@{#409916} [modify] https://crrev.com/18f89cf0b120afff95e064f437140bdbfb33ef36/chrome/browser/chromeos/accessibility/accessibility_manager.cc [modify] https://crrev.com/18f89cf0b120afff95e064f437140bdbfb33ef36/chrome/browser/extensions/component_loader.cc [modify] https://crrev.com/18f89cf0b120afff95e064f437140bdbfb33ef36/chrome/browser/extensions/component_loader.h
,
Aug 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03bc1f9c36c253bfd2ef0b53cf621a52128198f3 commit 03bc1f9c36c253bfd2ef0b53cf621a52128198f3 Author: dtseng <dtseng@chromium.org> Date: Tue Aug 09 15:50:06 2016 Revert of Move ChromeVox loading out of ComponentLoader. (patchset #3 id:40001 of https://codereview.chromium.org/2212853002/ ) Reason for revert: BUG= 635294 Repro: - run Chrome OS on linux with an existing profile and with the flag --login-manager - sign in or go through OOBE with ChromeVox on (and working) result: content scripts appear to be broken when logged in. I suspect user ChromeVox isn't loading. Original issue's description: > Move ChromeVox loading out of ComponentLoader. > > First, it isn't actually necessary to load ChromeVox from > ComponentLoader. This code is probably historical. For quite some time, > AccessibilityManager automatically loads ChromeVox on startup if the > flag is set (and automatically loads/unloads when the flag changes). > We should load it from only one place. Tested manually during startup > and when logging into a session, both work fine without this call. > > Second, I intend to refactor AccessibilityManager so that it can support > multiple accessibility component extensions. Rather than a separate function > in ComponentLoader to load each one, I think it'd make more sense for > AccessibilityManager to just call ComponentLoader with the appropriate > parameters like the extension id and path. > > BUG= 634233 > > Committed: https://crrev.com/18f89cf0b120afff95e064f437140bdbfb33ef36 > Cr-Commit-Position: refs/heads/master@{#409916} TBR=rdevlin.cronin@chromium.org,dmazzoni@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 634233 Review-Url: https://codereview.chromium.org/2229833002 Cr-Commit-Position: refs/heads/master@{#410685} [modify] https://crrev.com/03bc1f9c36c253bfd2ef0b53cf621a52128198f3/chrome/browser/chromeos/accessibility/accessibility_manager.cc [modify] https://crrev.com/03bc1f9c36c253bfd2ef0b53cf621a52128198f3/chrome/browser/extensions/component_loader.cc [modify] https://crrev.com/03bc1f9c36c253bfd2ef0b53cf621a52128198f3/chrome/browser/extensions/component_loader.h
,
Sep 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c0132079532714ac3628568ef3ecd3826a39c18 commit 2c0132079532714ac3628568ef3ecd3826a39c18 Author: dmazzoni <dmazzoni@chromium.org> Date: Wed Sep 07 19:59:37 2016 Re-land: Move ChromeVox loading out of ComponentLoader. Original changelist: http://crrev.com/2212853002 This change landed and was then reverted because it broke ChromeVox when logging in when it was already enabled on the login screen. The issue was that AccessibilityManager needed to add the component extension to each profile. The immediate benefit is that ComponentLoader doesn't need to depend on AccessibilityManager - previously they both depended on each other. The end goal is to refactor AccessibilityManager so that it can support multiple accessibility component extensions. Rather than a separate function in ComponentLoader to load each one, I think it'd make more sense for AccessibilityManager to just call ComponentLoader with the appropriate parameters like the extension id and path. BUG= 634233 TEST=Toggle ChromeVox on/off from login screen, logged-in session, and guest session. Enable ChromeVox in login screen and log in, ensure ChromeVox remains on. Try with both ChromeVox Classic and Next. Review-Url: https://codereview.chromium.org/2291133005 Cr-Commit-Position: refs/heads/master@{#417038} [modify] https://crrev.com/2c0132079532714ac3628568ef3ecd3826a39c18/chrome/browser/chromeos/accessibility/accessibility_manager.cc [modify] https://crrev.com/2c0132079532714ac3628568ef3ecd3826a39c18/chrome/browser/extensions/component_loader.cc [modify] https://crrev.com/2c0132079532714ac3628568ef3ecd3826a39c18/chrome/browser/extensions/component_loader.h
,
Sep 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/337c5afe3e3c93faa54c445355dc95299d0d73ef commit 337c5afe3e3c93faa54c445355dc95299d0d73ef Author: dmazzoni <dmazzoni@chromium.org> Date: Tue Sep 13 23:08:57 2016 Factor the extension loading code out of AccessibilityManager Create a new class, AccessibilityExtensionLoader, and pull all of the code that currently loads ChromeVox into it. This is meant to be a pure refactoring change with no user-perceivable change in functionality. If any change in behavior traces back to this change, that's a bug. The purpose of this change is so that AccessibilityManager can be extended to load additional accessibility component extensions other than ChromeVox. BUG= 634233 TBR=oshima@chromium.org TEST=Toggle ChromeVox on login screen, logged-in screen, guest screen, and lock screen. Test logging in with ChromeVox disabled and enabled. Review-Url: https://codereview.chromium.org/2212863002 Cr-Commit-Position: refs/heads/master@{#418401} [modify] https://crrev.com/337c5afe3e3c93faa54c445355dc95299d0d73ef/chrome/browser/chromeos/BUILD.gn [add] https://crrev.com/337c5afe3e3c93faa54c445355dc95299d0d73ef/chrome/browser/chromeos/accessibility/accessibility_extension_loader.cc [add] https://crrev.com/337c5afe3e3c93faa54c445355dc95299d0d73ef/chrome/browser/chromeos/accessibility/accessibility_extension_loader.h [modify] https://crrev.com/337c5afe3e3c93faa54c445355dc95299d0d73ef/chrome/browser/chromeos/accessibility/accessibility_manager.cc [modify] https://crrev.com/337c5afe3e3c93faa54c445355dc95299d0d73ef/chrome/browser/chromeos/accessibility/accessibility_manager.h
,
Sep 19 2016
,
Oct 7 2016
,
Oct 28 2016
As per #6 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dmazz...@chromium.org
, Aug 4 2016