Android accessibility actions inside iframes not sent to right frame

Android accessibility actions inside iframes not sent to right frame

When sending a message from the BrowserAccessibilityManagerAndroid to the
render process to perform some accessibility action on a node, we need to
send it to the right frame by calling the delegate function on the node's
BrowserAccessibilityManager. We were incorrectly calling it on the current
BrowserAccessibilityManager, which is only the same for elements in the
root frame.

BUG=562757
Committed: https://crrev.com/909ee6ee26be0ad7f89e07806c96f142328e0993
Cr-Commit-Position: refs/heads/master@{#422887}

[dmazzoni, 2016-10-04 17:30:49]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[dmazzoni, 2016-10-04 17:30:58]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 17:31:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Tseng, 2016-10-04 17:49:34]

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:602:
node->manager()->SetFocus(*node);
I'm not quite sure I understand the issue. Is the
BrowserAccessibilityManagerAndroid here not scoped to the frame? If not, then
what does the id mean as id's are only unique within a single tree, right?

Also, don't you need to do something similar for all calls to the manager e.g.
SetFocus -> GetRoot()->manager()->SetFocus below for Blur.

[David Tseng, 2016-10-04 17:56:40]

lgtm

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:594:
node->manager()->DoDefaultAction(*node);
Optional: override each of the actions and make them resolve to the correct
manager.

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:602:
node->manager()->SetFocus(*node);
On 2016/10/04 17:49:34, David Tseng wrote:
> I'm not quite sure I understand the issue. Is the
> BrowserAccessibilityManagerAndroid here not scoped to the frame? If not, then
> what does the id mean as id's are only unique within a single tree, right?
> 
> Also, don't you need to do something similar for all calls to the manager e.g.
> SetFocus -> GetRoot()->manager()->SetFocus below for Blur.

This makes sense now after offline discusison.

[dmazzoni, 2016-10-04 17:57:45]

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/2395533002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:602:
node->manager()->SetFocus(*node);
On 2016/10/04 17:49:34, David Tseng wrote:
> I'm not quite sure I understand the issue. Is the
> BrowserAccessibilityManagerAndroid here not scoped to the frame? If not, then
> what does the id mean as id's are only unique within a single tree, right?

Yeah, IDs from BrowserAccessibilityManager.java are all unique
IDs. So GetFromUniqueID is doing the right thing.

> Also, don't you need to do something similar for all calls to the manager e.g.
> SetFocus -> GetRoot()->manager()->SetFocus below for Blur.

Blur is kind of a special case because we interpret blur as
setting focus to the root node of the root tree, so this is
okay.

Let me know if you see any other case I might have missed,
though!

[commit-bot: I haz the power, 2016-10-04 18:12:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 18:12:21]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2016-10-04 19:32:45]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-10-04 19:33:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 19:39:15]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-04 19:41:53]

Description was changed from

==========
Android accessibility actions inside iframes not sent to right frame

When sending a message from the BrowserAccessibilityManagerAndroid to the
render process to perform some accessibility action on a node, we need to
send it to the right frame by calling the delegate function on the node's
BrowserAccessibilityManager. We were incorrectly calling it on the current
BrowserAccessibilityManager, which is only the same for elements in the
root frame.

BUG=562757
==========

to

==========
Android accessibility actions inside iframes not sent to right frame

When sending a message from the BrowserAccessibilityManagerAndroid to the
render process to perform some accessibility action on a node, we need to
send it to the right frame by calling the delegate function on the node's
BrowserAccessibilityManager. We were incorrectly calling it on the current
BrowserAccessibilityManager, which is only the same for elements in the
root frame.

BUG=562757

Committed: https://crrev.com/909ee6ee26be0ad7f89e07806c96f142328e0993
Cr-Commit-Position: refs/heads/master@{#422887}
==========

[commit-bot: I haz the power, 2016-10-04 19:41:54]

Patchset 1 (id:??) landed as
https://crrev.com/909ee6ee26be0ad7f89e07806c96f142328e0993
Cr-Commit-Position: refs/heads/master@{#422887}