Use forceClickOnCurrentItem to mean doDefault action rather than performDefaultAction

Use forceClickOnCurrentItem to mean doDefault action rather than performDefaultAction

When navigating to a new site with the below steps, we accidentally force focus back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.

This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault| action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to mean doDefault. This matches classic behavior. performDefaultAction in classic simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and working.

BUG=605980
Committed: https://crrev.com/5f2b437436f41145c774d485602bd61edefc443c
Cr-Commit-Position: refs/heads/master@{#390248}

[David Tseng, 2016-04-22 17:48:59]

Description was changed from

==========
Do not perform default action on single-lined text fields when hitting enter in
Classic mode.
==========

to

==========
Do not perform default action on single-lined text fields when hitting enter in
Classic mode.

When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by only performing default action on text fields that are multiline.

TEST=navigate between sites via the above reproduction; ensure classic is on and
working.
==========

[David Tseng, 2016-04-22 17:48:59]

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

[David Tseng, 2016-04-22 17:49:58]

This one took a bit to figure out. Should definitely merge back to m51.

[David Tseng, 2016-04-22 17:52:57]

Description was changed from

==========
Do not perform default action on single-lined text fields when hitting enter in
Classic mode.

When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by only performing default action on text fields that are multiline.

TEST=navigate between sites via the above reproduction; ensure classic is on and
working.
==========

to

==========
Do not perform default action on single-lined text fields when hitting enter in
Classic mode.

When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by only performing default action on text fields that are multiline.

TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

[dmazzoni, 2016-04-22 22:04:27]

Great diagnosis of the issue!

For the fix, I don't quite follow how multiline is related.
Could we just prevent performDefaultAction on any text field?

To really avoid unintended consequences, how about if Enter binds to a different
action altogether like performEnterKeyAction, and then that action could do the
same as performDefaultAction for most roles, but do nothing for text fields.

[David Tseng, 2016-04-22 22:53:53]

Description was changed from

==========
Do not perform default action on single-lined text fields when hitting enter in
Classic mode.

When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by only performing default action on text fields that are multiline.

TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

to

==========
Use forceClickOnCurrentItem to mean doDefault action rather than
performDefaultAction


When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by 
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to
mean doDefault. This matches classic behavior. performDefaultAction in classic
simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

[David Tseng, 2016-04-22 23:12:59]

PTAL. Reviewed the classic commands and forceClickOnCurrentItem is the
command we want for |doDefault| action. |performDefaultAction| appears to
be used to support internal links; otherwise, it's just a no-op on
ChromeVox's part (left up to Chrome).

On Fri, Apr 22, 2016 at 3:04 PM, <dmazzoni@chromium.org> wrote:

> Great diagnosis of the issue!
>
> For the fix, I don't quite follow how multiline is related.
> Could we just prevent performDefaultAction on any text field?
>
> To really avoid unintended consequences, how about if Enter binds to a
> different
> action altogether like performEnterKeyAction, and then that action could
> do the
> same as performDefaultAction for most roles, but do nothing for text
> fields.
>
>
> https://codereview.chromium.org/1915603002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-04-27 23:17:23]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-04-27 23:17:23]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-04-27 23:17:23]

lgtm

[dmazzoni, 2016-04-27 23:17:23]

lgtm

lgtm

[commit-bot: I haz the power, 2016-04-27 23:17:55]

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

[commit-bot: I haz the power, 2016-04-28 00:01:17]

Description was changed from

==========
Use forceClickOnCurrentItem to mean doDefault action rather than
performDefaultAction


When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by 
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to
mean doDefault. This matches classic behavior. performDefaultAction in classic
simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

to

==========
Use forceClickOnCurrentItem to mean doDefault action rather than
performDefaultAction


When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by 
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to
mean doDefault. This matches classic behavior. performDefaultAction in classic
simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

[commit-bot: I haz the power, 2016-04-28 00:01:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-30 17:14:49]

Patchset 3 (id:??) landed as
https://crrev.com/5f2b437436f41145c774d485602bd61edefc443c
Cr-Commit-Position: refs/heads/master@{#390248}

[commit-bot: I haz the power, 2016-04-30 17:53:11]

Description was changed from

==========
Use forceClickOnCurrentItem to mean doDefault action rather than
performDefaultAction


When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.


This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by 
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to
mean doDefault. This matches classic behavior. performDefaultAction in classic
simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980
==========

to

==========
Use forceClickOnCurrentItem to mean doDefault action rather than
performDefaultAction

When navigating to a new site with the below steps, we accidentally force focus
back onto the omnibox.
- ctrl+l
- type an address
- hit enter

result:
we stay in compat mode and we see focus is on the omnibox.

This is because:
- when we place focus via ctrl+l, we switch to compat mode
- that results in a switch to the classic keymap
- the classic keymap binds the enter key to performDefaultAction
- in cvox2/background/backgorund.js, performDefaultAction does a |doDefault|
action which sends a click
- after entering an address, we "click" the address bar
- this is racey with the focus generated by the webview

Solve this by
using the forceClickOnCurrentItem command (which is mapped to Cvox+space) to
mean doDefault. This matches classic behavior. performDefaultAction in classic
simply meant placing focus correctly when encountering internal links.
TEST=navigate between sites via the above reproduction; ensure classic is on and
working.

BUG=605980

Committed: https://crrev.com/5f2b437436f41145c774d485602bd61edefc443c
Cr-Commit-Position: refs/heads/master@{#390248}
==========