Define a minimum time for double tap in ChromeVox.

Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/9e0ab467434123946f1cf936257af2fbbc9ca555
Cr-Commit-Position: refs/heads/master@{#404305}

[David Tseng, 2016-07-07 17:51:30]

Description was changed from

==========
Define a minimum time for double tap in ChromeVox.
==========

to

==========
Define a minimum time for double tap in ChromeVox.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-07-07 17:56:10]

Description was changed from

==========
Define a minimum time for double tap in ChromeVox.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling
sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-07-07 17:56:10]

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

[chromium-reviews, 2016-07-07 18:20:45]

--- a/chrome/browser/resources/chromeos/chromevox/common/key_util.js
>
> - if (prevTime > 0 && currTime - prevTime < 300) { // Double tap
> + var delta = currTime - prevTime;
> + if (prevTime > 0 && delta > 100 && delta < 300) { // Double tap
Are there system settings somewhere controlling double-tap min delay and 
max delay?
Perhaps we could re-use the mouse settings, even though it's for the 
keyboard. I remember there is some similar behavior in Windows.
If this sounds peculiar, perhaps we should use the keyboard repeat delay 
as the max delay for double tap.
Anyway, you could add a TODO for now if you want.


> --- 
>
a/chrome/browser/resources/chromeos/chromevox/cvox2/background/notifications.js
>
> * Runs notifications that should be shown for startup.
> */
> Notifications.onStartup = function() {
> // Only run on background page.
> if (document.location.href.indexOf('background.html') == -1)
What if I have a page calleded google.com/background.html, would this 
run there as well?
Perhaps you should use chrome.runtime.getBackgroundPage or some 
similarly sounding API and compare with the current page object?

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

[nektarios, 2016-07-07 18:21:08]

lgtm

[David Tseng, 2016-07-07 18:37:09]

On Thu, Jul 7, 2016 at 11:20 AM, 'Nektarios Paisios' via Chromium-reviews <
chromium-reviews@chromium.org> wrote:

> --- a/chrome/browser/resources/chromeos/chromevox/common/key_util.js
>
>
> - if (prevTime > 0 && currTime - prevTime < 300) { // Double tap
> + var delta = currTime - prevTime;
> + if (prevTime > 0 && delta > 100 && delta < 300) { // Double tap
>
> Are there system settings somewhere controlling double-tap min delay and
> max delay?
> Perhaps we could re-use the mouse settings, even though it's for the
> keyboard. I remember there is some similar behavior in Windows.
> If this sounds peculiar, perhaps we should use the keyboard repeat delay
> as the max delay for double tap.
> Anyway, you could add a TODO for now if you want.
>
> Good suggestion; we don't have access to the system settings/preferences
> in this context, so we need to define some sane defaults.
>


> --- a/chrome/browser/resources/chromeos/chromevox/cvox2/
> background/notifications.js
>
> * Runs notifications that should be shown for startup.
> */
> Notifications.onStartup = function() {
> // Only run on background page.
> if (document.location.href.indexOf('background.html') == -1)
>
> What if I have a page calleded google.com/background.html, would this run
> there as well?
> Perhaps you should use chrome.runtime.getBackgroundPage or some similarly
> sounding API and compare with the current page object?
>
> This file can only be run on the three existing background pages ChromeVox
ships with optionts.html, panel.html or background.html. It doesn't get
injected into any pages as a content script.


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

[David Tseng, 2016-07-07 21:20:53]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-07-07 21:20:54]

The patchset sent to the CQ was uploaded after l-g-t-m from nektar@chromium.org
Link to the patchset: https://codereview.chromium.org/2123973004/#ps20001
(title: "Fix tests.")

[commit-bot: I haz the power, 2016-07-07 21:21:52]

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-07-07 23:23:58]

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

[commit-bot: I haz the power, 2016-07-07 23:23:59]

Try jobs failed on following builders:
  mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)

[David Tseng, 2016-07-08 01:47:29]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2016-07-08 01:47:55]

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-07-08 01:52:50]

Description was changed from

==========
Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling
sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling
sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-08 01:52:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-08 01:52:53]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-08 01:54:21]

Description was changed from

==========
Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling
sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Define a minimum time for double tap in ChromeVox.

TEST=flash Chromebook; hold search key and verify we're not repeatedly toggling
sticky mode.
BUG=617748
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/9e0ab467434123946f1cf936257af2fbbc9ca555
Cr-Commit-Position: refs/heads/master@{#404305}
==========

[commit-bot: I haz the power, 2016-07-08 01:54:22]

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