Parse meta tag that disables ChromeVox content script and forces compat mode.

Parse meta tag that disables ChromeVox content script and forces compat mode.

Users can now add this HTML snippet to their document HEAD to ensure
that ChromeVox only uses Next or Compat mode with that url.

<meta name="chromevox" content-script="no">

BUG=619736
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[dmazzoni, 2016-06-15 22:34:04]

Description was changed from

==========
Parse meta tag that disables ChromeVox content script and forces compat mode.

Users can now add this HTML snippet to their document HEAD to ensure
that ChromeVox only uses Next or Compat mode with that url.

<meta name="chromevox" content-script="no">

BUG=619736
==========

to

==========
Parse meta tag that disables ChromeVox content script and forces compat mode.

Users can now add this HTML snippet to their document HEAD to ensure
that ChromeVox only uses Next or Compat mode with that url.

<meta name="chromevox" content-script="no">

BUG=619736
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2016-06-15 22:34:26]

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

[David Tseng, 2016-06-15 22:50:17]

lgtm

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js
(right):

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:88:
document.querySelectorAll('head meta[name="chromevox"]').forEach(
Could you do: document.head.querySelectorAll(...

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:90:
if (meta.getAttribute('content-script').toLowerCase() == 'no') {
Check getAttribute('content-script') is non-null.

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

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1092:
this.classicBlacklist_.push(url);
Should be a dictionary/set since we don't want dups.

[dmazzoni, 2016-06-16 20:00:06]

Feedback addressed, and also made it so that it only
disables Classic in the current tab, rather than
killing Classic everywhere when switching to compat
mode for just one url. This was a bug if you used
#chromevox_next_test, too.

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js
(right):

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:88:
document.querySelectorAll('head meta[name="chromevox"]').forEach(
On 2016/06/15 22:50:16, David Tseng wrote:
> Could you do: document.head.querySelectorAll(...

Done.

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:90:
if (meta.getAttribute('content-script').toLowerCase() == 'no') {
On 2016/06/15 22:50:16, David Tseng wrote:
> Check getAttribute('content-script') is non-null.

Done.

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

https://codereview.chromium.org/2074513002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1092:
this.classicBlacklist_.push(url);
On 2016/06/15 22:50:17, David Tseng wrote:
> Should be a dictionary/set since we don't want dups.

Done.

[David Tseng, 2016-06-16 21:36:33]

https://codereview.chromium.org/2074513002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2074513002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:255:
this.disableClassicChromeVox_(tabs);
If setting force next mode, we do want to disable all running instances of
classic (including those not in a tab).

[dmazzoni, 2016-06-17 22:20:38]

https://codereview.chromium.org/2074513002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2074513002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:255:
this.disableClassicChromeVox_(tabs);
On 2016/06/16 21:36:33, David Tseng wrote:
> If setting force next mode, we do want to disable all running instances of
> classic (including those not in a tab).

Good catch. Take a look.

[David Tseng, 2016-06-20 16:26:08]

lgtm

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js
(right):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:98:
window.console.log('Disabling ChromeVox content script due to meta tag.');
nit: remove

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(left):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1025:
command: 'killChromeVox'
Same object; write it once at the top and in both branch of the if/else.

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1040:
disableClassicChromeVox_: function(opt_tabs) {
Note for possibly a future refactoring cl: background is getting pretty large.
We should move all methods that don't depend on background instance members to
another file.

[dmazzoni, 2016-06-20 20:05:40]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-06-20 20:05:41]

The patchset sent to the CQ was uploaded after l-g-t-m from dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2074513002/#ps60001
(title: "Address last feedback")

[commit-bot: I haz the power, 2016-06-20 20:06:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2074513002/60001

[dmazzoni, 2016-06-20 20:06:24]

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js
(right):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/init_document.js:98:
window.console.log('Disabling ChromeVox content script due to meta tag.');
On 2016/06/20 16:26:08, David Tseng wrote:
> nit: remove

Done.

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(left):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1025:
command: 'killChromeVox'
On 2016/06/20 16:26:08, David Tseng wrote:
> Same object; write it once at the top and in both branch of the if/else.

Done.

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/2074513002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:1040:
disableClassicChromeVox_: function(opt_tabs) {
On 2016/06/20 16:26:08, David Tseng wrote:
> Note for possibly a future refactoring cl: background is getting pretty large.
> We should move all methods that don't depend on background instance members to
> another file.

Acknowledged.

[commit-bot: I haz the power, 2016-06-20 21:43:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-20 21:45:31]

Description was changed from

==========
Parse meta tag that disables ChromeVox content script and forces compat mode.

Users can now add this HTML snippet to their document HEAD to ensure
that ChromeVox only uses Next or Compat mode with that url.

<meta name="chromevox" content-script="no">

BUG=619736
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Parse meta tag that disables ChromeVox content script and forces compat mode.

Users can now add this HTML snippet to their document HEAD to ensure
that ChromeVox only uses Next or Compat mode with that url.

<meta name="chromevox" content-script="no">

BUG=619736
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-06-20 21:45:33]

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