Make contracted braille input work in more contexts.

Make contracted braille input work in more contexts.

Before, when entering contracted (gradd 2) braille, backtracking was done
using the chrome.input.ime.deleteSurroundingText API.  This made text entry
visible in the text field immediately.  Unfortunately, some contexts don't
handle programmatic deletion well (Google Docs' editor is an example).

The new approach is to not commit the text ot the edit field until a word
has been entered or a non-braille key is entered. To make the user experience
better for the braille user, entered braille cells are shown on the
display right before the curent selection so that the user gets immediate
feedback when typing on the braille dipslay.

This approach has drawbacks as well. For example, clicking with a mouse
while typing could move focus before the IME has a chance to submit the
in-progress word. The assumption is that this would be less of a
problem in practice then not being able to use grade 2 at all in some
situations.

BUG=392854
Committed: https://crrev.com/c808864717a9641fd367af747763ce0935b1236f
Cr-Commit-Position: refs/heads/master@{#330711}

[Peter Lundblad, 2015-03-27 15:41:58]

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

[Peter Lundblad, 2015-03-27 15:41:58]

FYI, follow up to the braille input refactoring for motivation.
Soon ready.

[David Tseng, 2015-03-30 17:13:44]

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/braille_ime/braille_ime.js (right):

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:136: /** @type
{number} */
nit: @private; also, perhaps combine these two members into one object.

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:455: * @param
{string} requestId Key even request id.
nit: Key event

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:457: * @param
{boolean} response Wether the IME handled theevent.
nit: whether ... the event

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:468: * Calls the
correspnding IME API function and stores the current
nit: corresponding

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:482: * Commits the
last set composition if it mathces the given context id.
nit: matches

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/braille_ime/braille_ime.js:486: if
(opt_contextID === undefined ||
Why is this optional?

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/braille/braille_input_handler.js
(right):

https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/braille/braille_input_handler.js:266:
useComposition = false;
This looks wrong.

[Peter Lundblad, 2015-05-05 15:18:14]

Hi,

Still not ready for review, just responding to old comments.


dtseng@chromium.org writes:
> 
> 
> 
> 
>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> File chrome/browser/resources/chromeos/braille_ime/braille_ime.js
> (right):
> 
>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:136: /**
> @type {number} */
> nit: @private; also, perhaps combine these two members into one object.

Done

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:455: *
> @param {string} requestId Key even request id.
> nit: Key event

Done.

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:457: *
> @param {boolean} response Wether the IME handled theevent.
> nit: whether ... the event

Done.

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:468: *
> Calls the correspnding IME API function and stores the current
> nit: corresponding

Done.

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:482: *
> Commits the last set composition if it mathces the given context id.
> nit: matches

Done.

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/braille_ime/braille_ime.js:486: if
> (opt_contextID === undefined ||
> Why is this optional?

It used to be optional because if it is called locally, then the context
id passed in would be the one stored in the object.  Changed the callee in the
key handler
to just pass the current context id in as well for clarity.

>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
> File
> chrome/browser/resources/chromeos/chromevox/braille/braille_input_handler.js
> (right):
> 
>
https://codereview.chromium.org/1039703002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/braille/braille_input_handler.js:266:
> useComposition = false;
> This looks wrong.

This was for debugging, hence the WIP;)

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Peter Lundblad, 2015-05-11 20:53:48]

David, ready for review and perhaps even a test drive. I'd like your feedback on
this since I am not a good tester of the UX when I know what to expect;)

[David Tseng, 2015-05-11 22:45:03]

A few issues observed while using this with Docs:
- while typing input, if you press an arrow key, the untranslated string
appears to be inserted.
- routing keys don't work
- very hard to edit an existing word (seems buggy). For example, given
"is_" where _ is the caret, type "n't" results in "isn.t_".
- backspace seems to no longer work on my display (Focus 40).
- more of a preference, but I'd prefer we not expand words unless the caret
is under it.
- an interesting possibly translation error: "to    come" translate to <to
sign><com sign>e.


On Mon, May 11, 2015 at 1:53 PM, <plundblad@chromium.org> wrote:

> David, ready for review and perhaps even a test drive. I'd like your
> feedback on
> this since I am not a good tester of the UX when I know what to expect;)
>
> https://codereview.chromium.org/1039703002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2015-05-19 19:05:48]

lgtm

[Peter Lundblad, 2015-05-20 07:47:36]

The CQ bit was checked by plundblad@chromium.org

[Peter Lundblad, 2015-05-20 07:47:36]

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/1039703002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2015-05-20 07:48:00]

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

[commit-bot: I haz the power, 2015-05-20 07:51:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-20 07:52:47]

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