Implement auto read on load complete

Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic read/continuous read on load. Defaults to off.

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

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

[David Tseng, 2016-12-13 22:49:10]

Description was changed from

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

BUG=
==========

to

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

[David Tseng, 2016-12-13 22:49:38]

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

[David Tseng, 2016-12-13 22:59:24]

Description was changed from

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

to

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

[dmazzoni, 2016-12-15 17:07:02]

lgtm with some ideas

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

https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/desktop_automation_handler.js:300:
// If auto read is set, skip focus recovery and start reading from the top.
Have you considered doing auto-read only if there's no focus to recover?

I could be wrong but I think JAWS might do something more nuanced, because
for a simple document it reads from the top, but on apps with autofocus it
jumps right there, and I think it may be smart about reloads too.

I'm thinking good options would be:

Read the page when it finishes loading:
[ ] Never
[ ] Only the first time and when nothing has focus
[ ] Always

https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
(right):

https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd:478:
Automatically read a page after it finishes loading.
I'd maybe say "Automatically read page" or "Automatically read each page" or
"Automatically read current page"

[David Tseng, 2016-12-15 17:22:01]

On Thu, Dec 15, 2016 at 9:07 AM, <dmazzoni@chromium.org> wrote:
lgtm with some ideas



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

https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/desktop_automation_handler.js:300:
// If auto read is set, skip focus recovery and start reading from the
top.
Have you considered doing auto-read only if there's no focus to recover?

I could be wrong but I think JAWS might do something more nuanced,
because
for a simple document it reads from the top, but on apps with autofocus
it
jumps right there, and I think it may be smart about reloads too.

I'm thinking good options would be:

Read the page when it finishes loading:
[ ] Never
[ ] Only the first time and when nothing has focus
[ ] Always

This looks way too complicated. Those who like auto read tend to be novice
users and I'd vote for consistency over smart.


Our load completes also are not super strict, so I suspect this will
uncover more bugs. If you start tabbing, I believe at one point, we were
automatically sending load complete. On refresh, sometimes we don't even
send load complete.


https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
(right):

https://codereview.chromium.org/2577473003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd:478:
Automatically read a page after it finishes loading.
I'd maybe say "Automatically read page" or "Automatically read each
page" or "Automatically read current page"

Removed 'a' is clearer (does not imply auto reading background pages I
guess), so done.
https://codereview.chromium.org/2577473003/
-- 
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-12-15 23:11:53]

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

[commit-bot: I haz the power, 2016-12-15 23:12:47]

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, 2016-12-16 02:39:21]

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

[commit-bot: I haz the power, 2016-12-16 02:39:21]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2016-12-16 15:22:33]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-12-16 15:22:34]

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/2577473003/#ps20001
(title: "Implement auto read on load complete")

[commit-bot: I haz the power, 2016-12-16 15:22:57]

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-12-16 15:27:38]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481901753849540,
"parent_rev": "a5995df0c270aad0d41051c8c6a4b023e661aadf", "commit_rev":
"9e1783bf54ea95ac6a2f2c3a835da14bb86bb356"}

[commit-bot: I haz the power, 2016-12-16 15:28:05]

Description was changed from

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

to

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

Review-Url: https://codereview.chromium.org/2577473003
==========

[commit-bot: I haz the power, 2016-12-16 15:28:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-16 15:31:58]

Description was changed from

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

Review-Url: https://codereview.chromium.org/2577473003
==========

to

==========
Implement auto read on load complete

This cl adds an option/preference for a user to trigger automatic
read/continuous read on load. Defaults to off.

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

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

[commit-bot: I haz the power, 2016-12-16 15:31:59]

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