Switch to selected 8-dot braille table in email and url text fields

Switch to selected 8-dot braille table in email and url text fields

BUG=707933
TEST=verify manually email and url type inputs have 8-dot i/o. Verify that switching away from these fields reverts back to user selected table. OutputE2ETest revision for automation.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2813743002
Cr-Commit-Position: refs/heads/master@{#464066}
Committed: https://chromium.googlesource.com/chromium/src/+/e49774a1cca97ac55ee4f9af730cc36cb4b79015

[David Tseng, 2017-04-10 20:27:12]

Description was changed from

==========
Switch to selected 8-dot braille table in email and url text fields

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

to

==========
Switch to selected 8-dot braille table in email and url text fields

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

[David Tseng, 2017-04-10 20:28:45]

Description was changed from

==========
Switch to selected 8-dot braille table in email and url text fields

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

to

==========
Switch to selected 8-dot braille table in email and url text fields

BUG=707933
TEST=verify manually email and url type inputs have 8-dot i/o. Verify that
switching away from these fields reverts back to user selected table.
OutputE2ETest revision for automation.

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

[David Tseng, 2017-04-10 20:28:46]

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

[dmazzoni, 2017-04-10 22:47:33]

Let's think about a solution for 6-dot-only displays like Canute.

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

https://codereview.chromium.org/2813743002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:47:
localStorage['brailleTable'] = localStorage['brailleTable8'];
Maybe I'm not understanding what's happening here, but it looks like if you were
to exit Chrome after tabbing to a url field, your braille settings would be
changed to
match your 8-dot table and wouldn't be restored.

How about a function somewhere that retrieves the current braille table, rather
than
the braille code accessing it from localStorage directly? Then that code could
check
whether we're focused on a text field or not, but the actual underlying setting
wouldn't change.

I don't have a strong preference as to the exact solution but I would prefer
some
cleaner solution that doesn't require a save/restore, maybe refactoring
braille_translator_manager a bit.

https://codereview.chromium.org/2813743002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:49:
window['braille_translator_manager'].refresh();
Why is this hack needed here, is this for Closure?

[David Tseng, 2017-04-11 17:38:39]

In terms of 6-dot only displays, we need to revisit that topic at a higher level
since various other things rely upon 8 dot output including the cursor (dots
7-8). 8-dot input might not be an issue if the display has 8-dot perkins keys.

PTAL

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

https://codereview.chromium.org/2813743002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:47:
localStorage['brailleTable'] = localStorage['brailleTable8'];
On 2017/04/10 22:47:33, dmazzoni wrote:
> Maybe I'm not understanding what's happening here, but it looks like if you
were
> to exit Chrome after tabbing to a url field, your braille settings would be
> changed to
> match your 8-dot table and wouldn't be restored.
> 
> How about a function somewhere that retrieves the current braille table,
rather
> than
> the braille code accessing it from localStorage directly? Then that code could
> check
> whether we're focused on a text field or not, but the actual underlying
setting
> wouldn't change.
> 
> I don't have a strong preference as to the exact solution but I would prefer
> some
> cleaner solution that doesn't require a save/restore, maybe refactoring
> braille_translator_manager a bit.

I extracted all code in refresh that relies upon localStorage. Note that
localStorage['brailleTable'] is the user selected table with 'brailleTable6' and
'brailleTable8' being the user selected variants for each type of table.

https://codereview.chromium.org/2813743002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:49:
window['braille_translator_manager'].refresh();
On 2017/04/10 22:47:33, dmazzoni wrote:
> Why is this hack needed here, is this for Closure?

Not really a hack since it's exported on window. Underlying issue is that
Classic background is itself never exported on window, so all members (and
submembers) are inaccessible. I made BrailleBackground a singleton and use that
to get the BTM.

[dmazzoni, 2017-04-11 17:46:19]

lgtm

https://codereview.chromium.org/2813743002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.cc (right):

https://codereview.chromium.org/2813743002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:708:
node_data->html_attributes.push_back(std::make_pair("type", "url"));
Doesn't need to be done in this change, but this is a clear
sign that we ought to make "input type" an accessibility attribute

[David Tseng, 2017-04-11 18:11:20]

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

[David Tseng, 2017-04-11 18:11:20]

+ thestig for c/b/u/v/o/omnibox_view_views.cc

[Lei Zhang, 2017-04-11 18:25:29]

On 2017/04/11 18:11:20, David Tseng wrote:
> + thestig for c/b/u/v/o/omnibox_view_views.cc

lgtm

You can ask chrome/browser/ui/views/omnibox/OWNERS, BTW.

[David Tseng, 2017-04-12 14:24:50]

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

[commit-bot: I haz the power, 2017-04-12 14:25:18]

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-04-12 16:19:50]

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

[commit-bot: I haz the power, 2017-04-12 16:19:51]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2017-04-12 17:01:18]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2017-04-12 17:01:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2813743002/#ps40001
(title: "Fixes corner cases (liblouis callbacks sometimes drop when called
quickly).")

[commit-bot: I haz the power, 2017-04-12 17:02:24]

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-04-12 17:28:00]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492016478046280,
"parent_rev": "dc448150e1780b9158ecce7cf8ee153a8f9f61cf", "commit_rev":
"e49774a1cca97ac55ee4f9af730cc36cb4b79015"}

[commit-bot: I haz the power, 2017-04-12 17:29:00]

Description was changed from

==========
Switch to selected 8-dot braille table in email and url text fields

BUG=707933
TEST=verify manually email and url type inputs have 8-dot i/o. Verify that
switching away from these fields reverts back to user selected table.
OutputE2ETest revision for automation.

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

to

==========
Switch to selected 8-dot braille table in email and url text fields

BUG=707933
TEST=verify manually email and url type inputs have 8-dot i/o. Verify that
switching away from these fields reverts back to user selected table.
OutputE2ETest revision for automation.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2813743002
Cr-Commit-Position: refs/heads/master@{#464066}
Committed:
https://chromium.googlesource.com/chromium/src/+/e49774a1cca97ac55ee4f9af730c...
==========

[commit-bot: I haz the power, 2017-04-12 17:29:01]

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