Settings to remap Backspace and Escape keys

Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/168ecd71dbd357efb815c2b8380a434331d4cae7
Cr-Commit-Position: refs/heads/master@{#414303}

[michaelpg, 2016-08-16 21:03:41]

Description was changed from

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
==========

to

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-08-16 21:16:19]

Description was changed from

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
==========

to

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-08-16 21:19:25]

michaelpg@chromium.org changed reviewers:
+ asvitkine@chromium.org

[michaelpg, 2016-08-16 21:19:25]

michaelpg@chromium.org changed required reviewers:
+ asvitkine@chromium.org, sadrul@chromium.org, stevenjb@chromium.org

[michaelpg, 2016-08-16 21:19:26]

PTAL!

asvitkine:
  tools/metrics/actions/actions.xml

stevenjb:
  chrome/browser/chromeos/preferences.cc
  chrome/browser/resources
  chrome/browser/ui

sadrul:
  chrome/browser/chromeos/events
  ui/base/ime/chromeos/ime_keyboard.h

kpschoedel for FYI/optional review: chrome/browser/chromeos/events

[kpschoedel, 2016-08-17 16:51:23]

event_rewriter lgtm

[stevenjb, 2016-08-17 22:35:30]

The webui lgtm.

It seems a little odd to let users remap esc or backspace to a different
modifier key, but I'm not that familiar with key remapping so I will leave the
review of that area to others.

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/events/event_rewriter.h:38: //   Diamond) according to
user preferences;
Are Esc/Backspace considered 'modifier keys'? The others don't typically do
things on their own (with the exception of caps lock/search, but that's kind of
a funky exception for chrome os).

[michaelpg, 2016-08-17 23:00:57]

michaelpg@chromium.org changed reviewers:
+ jwd@chromium.org
- asvitkine@chromium.org

[michaelpg, 2016-08-17 23:00:57]

michaelpg@chromium.org changed required reviewers:
+ jwd@chromium.org
- asvitkine@chromium.org

[michaelpg, 2016-08-17 23:00:58]

stevenjb: I clarified some comments around "modifiers", which are the only
non-rebase changes with the latest patchset (see
chrome/browser/chromeos/events/event_rewriter.*)

+jwd for tools/metrics/actions/actions.xml

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/events/event_rewriter.h:38: //   Diamond) according to
user preferences;
On 2016/08/17 22:35:30, stevenjb wrote:
> Are Esc/Backspace considered 'modifier keys'? The others don't typically do
> things on their own (with the exception of caps lock/search, but that's kind
of
> a funky exception for chrome os).

Technically, no. (Caps Lock is normally also not considered a modifier.) But
they're mappable to modifier keys, and in this particular implementation, make
the most sense to handle in the same place as modifier keys.

[stevenjb, 2016-08-17 23:04:08]

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2250473006/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/events/event_rewriter.h:38: //   Diamond) according to
user preferences;
On 2016/08/17 23:00:58, michaelpg wrote:
> On 2016/08/17 22:35:30, stevenjb wrote:
> > Are Esc/Backspace considered 'modifier keys'? The others don't typically do
> > things on their own (with the exception of caps lock/search, but that's kind
> of
> > a funky exception for chrome os).
> 
> Technically, no. (Caps Lock is normally also not considered a modifier.) But
> they're mappable to modifier keys, and in this particular implementation, make
> the most sense to handle in the same place as modifier keys.

OK, thanks for clarifying the comment.

[sadrul, 2016-08-18 03:59:12]

lgtm

[jwd, 2016-08-18 19:25:46]

The actions.xml change as is fine, but it won't be easy to answer the question
"how many users override these keys". Is there desire for that?

[michaelpg, 2016-08-18 19:35:39]

On 2016/08/18 19:25:46, jwd wrote:
> The actions.xml change as is fine, but it won't be easy to answer the question
> "how many users override these keys". Is there desire for that?

@jwd: That, or "how often do users override these keys", would be fine. Why does
this not answer the question?

I believe this counts the times that a user changes the key's mapping (even
changing it back to its default, which I think is fine). If we need another
metric like "how many users have special mappings", I'd think that would happen
outside the Options UI.

[michaelpg, 2016-08-19 20:16:25]

On 2016/08/18 19:35:39, michaelpg wrote:
> On 2016/08/18 19:25:46, jwd wrote:
> > The actions.xml change as is fine, but it won't be easy to answer the
question
> > "how many users override these keys". Is there desire for that?
> 
> @jwd: That, or "how often do users override these keys", would be fine. Why
does
> this not answer the question?
> 
> I believe this counts the times that a user changes the key's mapping (even
> changing it back to its default, which I think is fine). If we need another
> metric like "how many users have special mappings", I'd think that would
happen
> outside the Options UI.

jwd: friendly ping/does this LGTY?

[jwd, 2016-08-22 19:21:51]

On 2016/08/19 20:16:25, michaelpg wrote:
> On 2016/08/18 19:35:39, michaelpg wrote:
> > On 2016/08/18 19:25:46, jwd wrote:
> > > The actions.xml change as is fine, but it won't be easy to answer the
> question
> > > "how many users override these keys". Is there desire for that?
> > 
> > @jwd: That, or "how often do users override these keys", would be fine. Why
> does
> > this not answer the question?
> > 
> > I believe this counts the times that a user changes the key's mapping (even
> > changing it back to its default, which I think is fine). If we need another
> > metric like "how many users have special mappings", I'd think that would
> happen
> > outside the Options UI.
I guess it depends on how you're asking the questions. You'll always be boxed
into a time window. This many users did it in the last 28 days, or it was
overridden this many times a day/week over this period. User counts are harder
to aggregate over longer periods (28-days can be slow, and fail), counting the
events is easier for longer periods though, so you'll still be able to see how
many times this setting was used in the last few months.

Anyway, LGTM.
> 
> jwd: friendly ping/does this LGTY?

[michaelpg, 2016-08-25 02:15:07]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-08-25 02:15:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, kpschoedel@chromium.org, sadrul@chromium.org,
jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2250473006/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-25 02:15:51]

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-08-25 04:01:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-25 04:03:22]

Description was changed from

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Settings to remap Backspace and Escape keys

We can remap modifier keys in Settings, but not Backspace or Escape;
this adds settings to remap those keys and handles them in EventRewriter.

BUG=608625
R=kpschoedel@chromium.org,stevenjb@chromium.org,sadrul@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/168ecd71dbd357efb815c2b8380a434331d4cae7
Cr-Commit-Position: refs/heads/master@{#414303}
==========

[commit-bot: I haz the power, 2016-08-25 04:03:23]

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