Avoid getting stuck when calling focus() on iframe nodes.

Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea| with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being really focused (i.e. do not call focus() again).

Committed: https://crrev.com/ac9cb99e59ade93c139464a338900ab0b204fdf0
Cr-Commit-Position: refs/heads/master@{#371309}

[David Tseng, 2016-01-22 23:10:01]

Description was changed from

==========
Avoid getting stuck when calling focus() on iframe nodes.
==========

to

==========
Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea|
with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first
ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being
really focused (i.e. do not call focus() again).
==========

[David Tseng, 2016-01-22 23:10:01]

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

[dmazzoni, 2016-01-23 00:01:38]

lgtm

https://codereview.chromium.org/1621823003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1621823003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:506:
if (!actionNode.role == RoleType.iframe && actionNode.state.focused)
The order of precedence isn't obvious to me here, could you either put the thing
being negated in parens, or use a != operator?

[David Tseng, 2016-01-25 20:13:16]

Added test as well.

https://codereview.chromium.org/1621823003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1621823003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:506:
if (!actionNode.role == RoleType.iframe && actionNode.state.focused)
On 2016/01/23 00:01:38, dmazzoni wrote:
> The order of precedence isn't obvious to me here, could you either put the
thing
> being negated in parens, or use a != operator?

Done.

[David Tseng, 2016-01-25 20:13:28]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-01-25 20:13:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/1621823003/#ps20001
(title: "Add test.")

[commit-bot: I haz the power, 2016-01-25 20:16:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1621823003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1621823003/20001

[commit-bot: I haz the power, 2016-01-25 20:54:18]

Description was changed from

==========
Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea|
with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first
ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being
really focused (i.e. do not call focus() again).
==========

to

==========
Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea|
with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first
ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being
really focused (i.e. do not call focus() again).
==========

[commit-bot: I haz the power, 2016-01-25 20:54:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-25 20:55:39]

Description was changed from

==========
Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea|
with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first
ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being
really focused (i.e. do not call focus() again).
==========

to

==========
Avoid getting stuck when calling focus() on iframe nodes.

Given a focusable node |iframe| with role iframe and a focusable child |webarea|
with role webArea,

iframe.focus()

results in a focus event being fired on |webarea|.

When navigating backwards, since ChromeVox visits every node in depth-first
ordering (reversed in this case), the following occurs
- ChromeVox range is on |webarea|
- ChromeVox walks and matches on |iframe|
- Chromevox calls iframe.focus().
- focus event fires on |webarea|

result:
ChromeVox range goes to |webarea| again and gets read aloud/brailled.

expected:
ChromeVox range should be on the |iframe|.

We can achieve this by not calling focus on the iframe node itself.

Additionally, consider nodes that already have the focused state set as being
really focused (i.e. do not call focus() again).

Committed: https://crrev.com/ac9cb99e59ade93c139464a338900ab0b204fdf0
Cr-Commit-Position: refs/heads/master@{#371309}
==========

[commit-bot: I haz the power, 2016-01-25 20:55:39]

Patchset 2 (id:??) landed as
https://crrev.com/ac9cb99e59ade93c139464a338900ab0b204fdf0
Cr-Commit-Position: refs/heads/master@{#371309}