Make select-to-speak work with Google Docs.

Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=707639
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795633002
Cr-Commit-Position: refs/heads/master@{#472383}
Committed: https://chromium.googlesource.com/chromium/src/+/7491a7604bd447c70a09d3165e9df8c75e1ba7d1

[dmazzoni, 2017-04-03 05:12:55]

Description was changed from

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=593887
==========

to

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=593887
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-04-03 05:13:03]

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

[commit-bot: I haz the power, 2017-04-03 05:13:20]

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

[dmazzoni, 2017-04-03 05:15:42]

Description was changed from

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=593887
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=707639
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-04-03 05:15:43]

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

[commit-bot: I haz the power, 2017-04-03 06:04:58]

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

[commit-bot: I haz the power, 2017-04-03 06:04:59]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2017-04-04 17:56:44]

Friendly ping on this change

On Sun, Apr 2, 2017 at 10:20 PM <dmazzoni@chromium.org> wrote:

> Reviewers: David Tseng
> CL: https://codereview.chromium.org/2795633002/
>
> Description:
> Make select-to-speak work with Google Docs.
>
> Google Docs renders its document content using absolute-positioned html.
> There's no reason it shouldn't be accessible to something as simple as
> select-to-speak, but it puts aria-hidden=true around that whole block
> to enable screen reader support. Add a content script that changes the
> aria-hidden attribute when Select-to-speak loads.
>
> BUG=707639
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
>
> Affected files (+34, -1 lines):
> M chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> M chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> A
>
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
>
>
> Index: chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> diff --git a/chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> b/chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> index
>
8486295ae7097e4a568770d14c0e239466bfad1e..9be56005408aa0c91a49c14fe5aecc1373cb0ae5
> 100644
> --- a/chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> +++ b/chrome/browser/resources/chromeos/select_to_speak/BUILD.gn
> @@ -30,6 +30,7 @@ run_jsbundler("select_to_speak_copied_files") {
> "options.css",
> "options.html",
> "select_to_speak.js",
> + "select_to_speak_gdocs_script.js",
> "select_to_speak_main.js",
> "select_to_speak_options.js",
> "unchecked.png",
> Index:
> chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> diff --git
> a/chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> b/chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> index
>
a21a9303d0134bee52628eace1d126a9d330541c..b349596a0b849ea19f51a7fad9f1ce49f62f27bf
> 100644
> ---
> a/chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> +++
> b/chrome/browser/resources/chromeos/select_to_speak/manifest.json.jinja2
> @@ -24,5 +24,13 @@
> "desktop": true
> },
> "default_locale": "en",
> - "options_page": "options.html"
> + "options_page": "options.html",
> + "content_scripts": [
> + {
> + "matches": [ "https://docs.google.com/document*" ],
> + "js": [
> + "select_to_speak_gdocs_script.js"
> + ]
> + }
> + ]
> }
> Index:
>
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
> diff --git
>
a/chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
>
b/chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
> new file mode 100644
> index
>
0000000000000000000000000000000000000000..144c53f4feaf93e93bac0dea330e9c5512440b9a
> --- /dev/null
> +++
>
b/chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
> @@ -0,0 +1,24 @@
> +// Copyright 2017 The Chromium Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style license that can be
> +// found in the LICENSE file.
> +
> +var tries = 10;
> +
> +// Google Docs isn't compatible with Select-to-speak by default because
> +// in order to provide screen reader support, most of the rendered
> +// document has aria-hidden set on it, which has the side effect of
> +// hiding it from Select-to-speak too. Fix it by changing aria-hidden
> +// to false. Try multiple times in case the page isn't fully loaded when
> +// the content script runs.
> +function RemoveAriaHiddenFromGoogleDocsContent() {
> + var element = document.querySelector('.kix-zoomdocumentplugin-outer');
> + if (element) {
> + element.setAttribute('aria-hidden', 'false');
> + } else {
> + tries--;
> + if (tries > 0)
> + window.setTimeout(RemoveAriaHiddenFromGoogleDocsContent, 1000);
> + }
> +}
> +
> +RemoveAriaHiddenFromGoogleDocsContent();
>
>
> --
> 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, 2017-04-04 18:34:24]

https://codereview.chromium.org/2795633002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
(right):

https://codereview.chromium.org/2795633002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js:14:
var element = document.querySelector('.kix-zoomdocumentplugin-outer');
This looks really fragile. I thought we were moving away from doing things like
this?

At the very least, we should have some way of verifying that this works on the
live Docs domain.

Is this supposed to work in braille mode? When accessibility is on?Does a user
need to know to turn on accessibility via ctrl+alt+z (which conflicts with
ChromeVox)?

[dmazzoni, 2017-04-04 18:47:17]

On 2017/04/04 18:34:24, David Tseng wrote:
>
https://codereview.chromium.org/2795633002/diff/1/chrome/browser/resources/ch...
> File
>
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js
> (right):
> 
>
https://codereview.chromium.org/2795633002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/select_to_speak/select_to_speak_gdocs_script.js:14:
> var element = document.querySelector('.kix-zoomdocumentplugin-outer');
> This looks really fragile. I thought we were moving away from doing things
like
> this?

As for fragility, I could add a querySelector that finds aria-hidden somewhere
in
the right part of the DOM rather than a particular class name.

> At the very least, we should have some way of verifying that this works on the
> live Docs domain.
> 
> Is this supposed to work in braille mode? When accessibility is on?Does a user
> need to know to turn on accessibility via ctrl+alt+z (which conflicts with
> ChromeVox)?

This works whether Docs accessibility is on or off, and whether braille mode is
on or off. It doesn't break screen reader support, but it adds back the
static content to the page, which could potentially be confusing. But keep in
mind that we don't intend for Select-to-speak and ChromeVox to be used together.

[dmazzoni, 2017-04-04 18:58:55]

As discussed offline, I'll add a manual test
and look into running it on an fyi bot

[dmazzoni, 2017-04-19 21:01:04]

Manual test added, started email thread to ask about
fyi bot.

[dmazzoni, 2017-05-09 21:47:18]

David, does this test look good? I think the plan is
to put it into a new test suite that can run on an
fyi bot, if I do that is this good to go?

[David Tseng, 2017-05-10 16:26:37]

lgtm

lgtm with the FYI bot; thanks!

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc
(right):

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:121:
LOG(INFO) << "Waiting for Google Doc to load";
Remove logging.

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:125:
LOG(INFO) << "Doc loaded, triggering select-to-speak";
Remove logging.

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:137:
LOG(INFO) << "Utterance: '" << utterance << "'";
Remove logging.

[dmazzoni, 2017-05-16 19:47:47]

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

[commit-bot: I haz the power, 2017-05-16 19:49:04]

Dry run: 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, 2017-05-16 22:27:17]

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

[commit-bot: I haz the power, 2017-05-16 22:27:17]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dmazzoni, 2017-05-17 05:33:38]

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc
(right):

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:121:
LOG(INFO) << "Waiting for Google Doc to load";
On 2017/05/10 16:26:36, David Tseng wrote:
> Remove logging.

Done.

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:125:
LOG(INFO) << "Doc loaded, triggering select-to-speak";
On 2017/05/10 16:26:36, David Tseng wrote:
> Remove logging.

Done.

https://codereview.chromium.org/2795633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc:137:
LOG(INFO) << "Utterance: '" << utterance << "'";
On 2017/05/10 16:26:36, David Tseng wrote:
> Remove logging.

Done.

[dmazzoni, 2017-05-17 05:35:19]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2017-05-17 05:35:19]

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/2795633002/#ps40001
(title: "Ready to land")

[commit-bot: I haz the power, 2017-05-17 05:36:19]

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, 2017-05-17 08:25:03]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494999319267280,
"parent_rev": "f56cfdfce785ceb79ceb0264a20d0b117c694713", "commit_rev":
"7491a7604bd447c70a09d3165e9df8c75e1ba7d1"}

[commit-bot: I haz the power, 2017-05-17 08:25:15]

Description was changed from

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=707639
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Make select-to-speak work with Google Docs.

Google Docs renders its document content using absolute-positioned html.
There's no reason it shouldn't be accessible to something as simple as
select-to-speak, but it puts aria-hidden=true around that whole block
to enable screen reader support. Add a content script that changes the
aria-hidden attribute when Select-to-speak loads.

BUG=707639
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795633002
Cr-Commit-Position: refs/heads/master@{#472383}
Committed:
https://chromium.googlesource.com/chromium/src/+/7491a7604bd447c70a09d3165e9d...
==========

[commit-bot: I haz the power, 2017-05-17 08:25:16]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7491a7604bd447c70a09d3165e9d...