New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 753369 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Team-Accessibility

Blocked on:
issue 756707

Blocking:
issue 539536



Sign in to add a comment

SpannableInlineAutocomplete: accessibility does not work correctly

Project Member Reported by changwan@chromium.org, Aug 8 2017

Issue description

Device 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'.

 
Blocking: 539536
Project Member

Comment 2 by bugdroid1@chromium.org, 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

Blockedon: 756707
Labels: -M-61 ReleaseBlock-Stable
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Project Member

Comment 6 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
This got in M62.
Labels: Merge-TBD
[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.
Labels: -Merge-TBD
Status: Verified (was: Fixed)
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