New issue
Advanced search Search tips

Issue 652805 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Team-Accessibility

Blocked on:
issue 667559



Sign in to add a comment

Android accessibility getting stuck after closing SELECT element pop-up

Project Member Reported by dmazz...@chromium.org, Oct 4 2016

Issue description

See b/31240693 for one possible repro.

After closing a SELECT pop-up we got a focus event on the OPTION that was selected, which then caused subsequent navigation to be trapped within the options.

The fix is to map the focus event to the topmost leaf ancestor, which in this case would be the SELECT element.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7969d3a5bcf2af3fac57c3acef518298e8c48382

commit 7969d3a5bcf2af3fac57c3acef518298e8c48382
Author: dmazzoni <dmazzoni@chromium.org>
Date: Tue Oct 04 21:42:06 2016

Don't post events on internal nodes in the accessibility tree on Android.

Sometimes we get events on nodes in the internal accessibility tree that
are not exposed on that platform or under all circumstances. On Android
that led to an issue where after closing a SELECT pop-up, we got a focus
event on the OPTION and subsequent navigation was trapped inside that
control rather than outside it.

The fix is to map the focus event to the topmost leaf ancestor, which
in this case would be the SELECT element.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2397503002
Cr-Commit-Position: refs/heads/master@{#422944}

[modify] https://crrev.com/7969d3a5bcf2af3fac57c3acef518298e8c48382/content/browser/accessibility/browser_accessibility_manager_android.cc

Comment 2 by torne@chromium.org, Oct 5 2016

Labels: -Pri-3 M-54 ReleaseBlock-Stable Pri-1
Marking as M54 release blocker for now pending discussion on internal thread.
After playing with this a bit I want to refine it more. We should be calling PlatformChildCount() == 0 instead of PlatformIsLeaf() or we're doing the wrong thing for iframe elements, and also I realized this is almost the same as what BrowserAccessibility::GetClosestPlatformObject() was doing so we should fix it there.

[Bulk edit]

URGENT: This issue is marked as an RB-Stable for Android M54.  If it is going to block the release, it needs to be fixed ASAP, and merged back to branch 2840 by 5 PM PT Oct 11.

Please review and if you cannot get it fixed by then, ping me; if you don't believe you are the right owner, and you cannot find another one - please ping me.
dmazzoni@, your window for making changes here is extremely small, we need to spin a new build for this on Monday - let's chat when you're back.
Labels: Merge-Request-53 Merge-Request-54
OK, did more testing and ready to manually merge this.

Comment 7 by dimu@chromium.org, Oct 10 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.

Comment 8 by dimu@chromium.org, Oct 10 2016

Labels: -Merge-Request-54 Merge-Review-54
[Automated comment] Less than a week to go before stable on M54, we might already have a stable candidate build. Manual review required.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10 2016

Labels: merge-merged-2785_124
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b6eb6de84f9ccf47005fb59351de98b0719a877

commit 1b6eb6de84f9ccf47005fb59351de98b0719a877
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Oct 10 18:35:59 2016

Merge to M53: Don't post events on internal nodes in the accessibility tree on Android.

Sometimes we get events on nodes in the internal accessibility tree that
are not exposed on that platform or under all circumstances. On Android
that led to an issue where after closing a SELECT pop-up, we got a focus
event on the OPTION and subsequent navigation was trapped inside that
control rather than outside it.

The fix is to map the focus event to the topmost leaf ancestor, which
in this case would be the SELECT element.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2397503002
Cr-Commit-Position: refs/heads/master@{#422944}
(cherry picked from commit 7969d3a5bcf2af3fac57c3acef518298e8c48382)

Review URL: https://codereview.chromium.org/2410503002 .

Cr-Commit-Position: refs/branch-heads/2785_124@{#5}
Cr-Branched-From: 7f5781e9210f9b7c8914adbc018590eec1ede150-refs/branch-heads/2785@{#902}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/1b6eb6de84f9ccf47005fb59351de98b0719a877/content/browser/accessibility/browser_accessibility_manager_android.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d2ed04992c82f83d1bbbd39520f3e1867b6b28c

commit 6d2ed04992c82f83d1bbbd39520f3e1867b6b28c
Author: dmazzoni <dmazzoni@chromium.org>
Date: Mon Oct 10 18:38:51 2016

Use BrowserAccessibility::GetClosestPlatformObject consistently

We had two code blocks that were trying to do the same thing as
BrowserAccessibility::GetClosestPlatformObject(), but they should have
just called that function instead.

Also, the two code blocks had a bug in that they considered iframe elements
to be leaf nodes, which could cause problems. GetClosestPlatformObject()
doesn't have that bug because it only searches within the current frame,
so by switching to this function we fix that bug.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2408843002
Cr-Commit-Position: refs/heads/master@{#424196}

[modify] https://crrev.com/6d2ed04992c82f83d1bbbd39520f3e1867b6b28c/content/browser/accessibility/browser_accessibility_manager.cc
[modify] https://crrev.com/6d2ed04992c82f83d1bbbd39520f3e1867b6b28c/content/browser/accessibility/browser_accessibility_manager_android.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 10 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55a0dba38e5df7bea291c3aa0ce96b01892d74ec

commit 55a0dba38e5df7bea291c3aa0ce96b01892d74ec
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Oct 10 18:54:41 2016

Merge to M54: Don't post events on internal nodes in the accessibility tree on Android.

Sometimes we get events on nodes in the internal accessibility tree that
are not exposed on that platform or under all circumstances. On Android
that led to an issue where after closing a SELECT pop-up, we got a focus
event on the OPTION and subsequent navigation was trapped inside that
control rather than outside it.

The fix is to map the focus event to the topmost leaf ancestor, which
in this case would be the SELECT element.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2397503002
Cr-Commit-Position: refs/heads/master@{#422944}
(cherry picked from commit 7969d3a5bcf2af3fac57c3acef518298e8c48382)

Review URL: https://codereview.chromium.org/2405873004 .

Cr-Commit-Position: refs/branch-heads/2840@{#704}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/55a0dba38e5df7bea291c3aa0ce96b01892d74ec/content/browser/accessibility/browser_accessibility_manager_android.cc

Labels: -Merge-Review-53 -Merge-Review-54
I approved both of these offline.
Labels: Merge-Approved-55
Also, merge approved for final patch in c#10 to M55 (branch 2883).
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 13 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad7c976158eecc8bf288f1b2e5d1c19793e6630b

commit ad7c976158eecc8bf288f1b2e5d1c19793e6630b
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Oct 13 21:20:20 2016

Merge to M55: Use BrowserAccessibility::GetClosestPlatformObject consistently

We had two code blocks that were trying to do the same thing as
BrowserAccessibility::GetClosestPlatformObject(), but they should have
just called that function instead.

Also, the two code blocks had a bug in that they considered iframe elements
to be leaf nodes, which could cause problems. GetClosestPlatformObject()
doesn't have that bug because it only searches within the current frame,
so by switching to this function we fix that bug.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2408843002
Cr-Commit-Position: refs/heads/master@{#424196}
(cherry picked from commit 6d2ed04992c82f83d1bbbd39520f3e1867b6b28c)

Review URL: https://codereview.chromium.org/2420543003 .

Cr-Commit-Position: refs/branch-heads/2883@{#95}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ad7c976158eecc8bf288f1b2e5d1c19793e6630b/content/browser/accessibility/browser_accessibility_manager.cc
[modify] https://crrev.com/ad7c976158eecc8bf288f1b2e5d1c19793e6630b/content/browser/accessibility/browser_accessibility_manager_android.cc

Status: Fixed (was: Started)

Comment 16 by aluo@chromium.org, Oct 19 2016

Verified on Nexus 5x, NRD91N, 54.0.2840.68
Status: Verified (was: Fixed)
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad7c976158eecc8bf288f1b2e5d1c19793e6630b

commit ad7c976158eecc8bf288f1b2e5d1c19793e6630b
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Oct 13 21:20:20 2016

Merge to M55: Use BrowserAccessibility::GetClosestPlatformObject consistently

We had two code blocks that were trying to do the same thing as
BrowserAccessibility::GetClosestPlatformObject(), but they should have
just called that function instead.

Also, the two code blocks had a bug in that they considered iframe elements
to be leaf nodes, which could cause problems. GetClosestPlatformObject()
doesn't have that bug because it only searches within the current frame,
so by switching to this function we fix that bug.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2408843002
Cr-Commit-Position: refs/heads/master@{#424196}
(cherry picked from commit 6d2ed04992c82f83d1bbbd39520f3e1867b6b28c)

Review URL: https://codereview.chromium.org/2420543003 .

Cr-Commit-Position: refs/branch-heads/2883@{#95}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ad7c976158eecc8bf288f1b2e5d1c19793e6630b/content/browser/accessibility/browser_accessibility_manager.cc
[modify] https://crrev.com/ad7c976158eecc8bf288f1b2e5d1c19793e6630b/content/browser/accessibility/browser_accessibility_manager_android.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55a0dba38e5df7bea291c3aa0ce96b01892d74ec

commit 55a0dba38e5df7bea291c3aa0ce96b01892d74ec
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Oct 10 18:54:41 2016

Merge to M54: Don't post events on internal nodes in the accessibility tree on Android.

Sometimes we get events on nodes in the internal accessibility tree that
are not exposed on that platform or under all circumstances. On Android
that led to an issue where after closing a SELECT pop-up, we got a focus
event on the OPTION and subsequent navigation was trapped inside that
control rather than outside it.

The fix is to map the focus event to the topmost leaf ancestor, which
in this case would be the SELECT element.

BUG= 652805 

Review-Url: https://codereview.chromium.org/2397503002
Cr-Commit-Position: refs/heads/master@{#422944}
(cherry picked from commit 7969d3a5bcf2af3fac57c3acef518298e8c48382)

Review URL: https://codereview.chromium.org/2405873004 .

Cr-Commit-Position: refs/branch-heads/2840@{#704}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/55a0dba38e5df7bea291c3aa0ce96b01892d74ec/content/browser/accessibility/browser_accessibility_manager_android.cc

Blockedon: 667559

Sign in to add a comment