Issue metadata
Sign in to add a comment
|
SpannableInlineAutocomplete: accessibility does not work correctly |
||||||||||||||||||||||||
Issue descriptionDevice name: Pixel From "Settings > About Chrome" Application version: 61, 62 Operating system: any Android version URLs (if applicable): Steps to reproduce: (0) Turn on SpannableInlineAutocomplete option from about:flags. (1) Turn on talkback. (2) Go to omnibox. (3) Type 'google' on omnibox and type enter. (4) Go to omnibox again. (5) Clear the text. (6) Type 'g'. Expected result: The talkback should say 'g', 'oogle'. Actual result: The talkback says 'g', 'oogle', 'delete'.
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d4b7f0162af4c8507f5c5c22948befd3c8be380 commit 1d4b7f0162af4c8507f5c5c22948befd3c8be380 Author: Changwan Ryu <changwan@chromium.org> Date: Thu Aug 17 16:25:36 2017 Fix accessibility for the new omnibox model Because we add and remove autocomplete text for each IME operation, we currently send more accessibility events than are necessary. This CL imposes a new accessibility model as follows: 1) Suppress accessibility events in batch edit. 2) Send accessibility events at the end of batch edit or at the end of any change outside batch edit - which coincides with notification. 2.1) Send text changed event only if there was deletion or replacement. 2.2) Send text selection changed event when selection changes. 3) Send autocomplete as text changed event. This was needed especially because of the new deletion behavior. An alternative considered: override sendAccessibilityEventUnchecked and queue the events and send them at once at the end of batch edit or at the end of a change outside batch edit. The problem is that it cannot fully control deletion inside a batch edit. Also, life cycle management is a hassle. This CL also adds robolectric tests for accessibility on omnibox. Enabling accessibility events on robolectric is quite challenging and requires multiple steps: 1) Enable it through ShadowAccessibilityManager. 2) Pump sendAccessibilityEvent() -> sendAccessibilityEventUnchecked(). Step 2) is needed because of a static check inside AccessibilityManager. Then we can listen for onPopulateAccessibilityEvent() which has the populated accessibility event and verify it. BUG= 753369 Change-Id: I017d0f1ce6ac414402162ca9222e9d7083bf430a Reviewed-on: https://chromium-review.googlesource.com/606447 Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Changwan Ryu <changwan@chromium.org> Cr-Commit-Position: refs/heads/master@{#495190} [modify] https://crrev.com/1d4b7f0162af4c8507f5c5c22948befd3c8be380/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java [modify] https://crrev.com/1d4b7f0162af4c8507f5c5c22948befd3c8be380/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModel.java [modify] https://crrev.com/1d4b7f0162af4c8507f5c5c22948befd3c8be380/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java [modify] https://crrev.com/1d4b7f0162af4c8507f5c5c22948befd3c8be380/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java [modify] https://crrev.com/1d4b7f0162af4c8507f5c5c22948befd3c8be380/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java
,
Aug 18 2017
,
Aug 21 2017
,
Aug 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d82b8f39db52b7b77840c0e05386f6f3cc3fd5ee commit d82b8f39db52b7b77840c0e05386f6f3cc3fd5ee Author: Changwan Ryu <changwan@chromium.org> Date: Wed Aug 30 21:24:04 2017 Fix accessibility and test for the new omnibox model The previous attempt did not fully fix the accessibility because of a confusion caused by an error in robolectric ( crbug.com/756707 ). Specifically, ChangeWatcher#beforeTextChanged() and onTextChanged() in TextView.java check AccessibilityManager.getInstance().isEnabled() but ShadowAccessibilityManager.getInstance() just creates a new instance with isEnabled initialized to false. Therefore, setEnabled() was not working. As a result, TYPE_VIEW_TEXT_CHANGED event was sent out without being detected. As it may take longer to fix the root cause in robolectric, and also in order to help version upgrade, I am trying to remove test dependency by introducing a static switch named TEST_ACCESSIBILITY. I have tested against https://chromium-review.googlesource.com/c/external/robolectric/+/620112 to ensure that the new test works with the fix. Also, I noticed that InOrder#verifyNoMoreInteractions() does not catch all the missing verify statements, so added call counting checks. Now with the test fix and static switch, I am adding TYPE_VIEW_TEXT_CHANGED which make the accessibility event sequences very similar to those from the old model. BUG= 753369 Change-Id: I6435a6c662c42e1a7896fe74f9d44c3c6bf5506b Reviewed-on: https://chromium-review.googlesource.com/630498 Commit-Queue: Changwan Ryu <changwan@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#498613} [modify] https://crrev.com/d82b8f39db52b7b77840c0e05386f6f3cc3fd5ee/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java [modify] https://crrev.com/d82b8f39db52b7b77840c0e05386f6f3cc3fd5ee/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd7b13bd96b4fab786b24cc98cb9c42fd3905e6b commit bd7b13bd96b4fab786b24cc98cb9c42fd3905e6b Author: Changwan Ryu <changwan@chromium.org> Date: Thu Aug 31 23:14:21 2017 Enable accessibility tests for AutocompleteEditText Sets TEST_ACCESSIBILITY to true, and picks up the fix in robolectric. BUG= 756707 , 753369 Change-Id: Ida291a45baf942c19dfdc543578b445d1e4d7858 Reviewed-on: https://chromium-review.googlesource.com/643336 Reviewed-by: Ted Choc <tedchoc@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Changwan Ryu <changwan@chromium.org> Cr-Commit-Position: refs/heads/master@{#499065} [modify] https://crrev.com/bd7b13bd96b4fab786b24cc98cb9c42fd3905e6b/DEPS [modify] https://crrev.com/bd7b13bd96b4fab786b24cc98cb9c42fd3905e6b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java
,
Sep 5 2017
This got in M62.
,
Sep 5 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-62; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-62 label, otherwise remove Merge-TBD label. Thanks.
,
Sep 5 2017
,
Sep 6 2017
Now its reading as 'g', 'oogle'. Tested on latest M62: 62.0.3202.9. Hence closing this issue. Thanks |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by changwan@chromium.org
, Aug 8 2017