Short-term fix for CapsLock remapping on chromebooks with external keyboard

Change CapsLock remapping pref to not be syncable, so each device can have
its own value.  Update EventRewriter to honor local CapsLock pref setting,
even if the UI doesn't show CapsLock in keyboard dialog.

BUG=167237
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=198999

[David Roche, 2013-03-22 00:08:16]

Yusuke, here is the CL we've been discussing.

Alex, please see the bug for more background on this change.

[Yusuke Sato, 2013-03-22 18:42:21]

https://codereview.chromium.org/13008005/diff/1/chrome/browser/chromeos/prefe...
File chrome/browser/chromeos/preferences.cc (right):

https://codereview.chromium.org/13008005/diff/1/chrome/browser/chromeos/prefe...
chrome/browser/chromeos/preferences.cc:239:
registry->RegisterIntegerPref(prefs::kLanguageRemapCapsLockKeyTo,
I think we need to rename kLanguageRemapCapsLockKeyTo in
chrome/common/pref_names.cc and
chrome/browser/resources/options/chromeos/keyboard_overlay.html to
"settings.language.remap_caps_lock_key_to_r2" to avoid the scenario I wrote in
https://code.google.com/p/chromium/issues/detail?id=167237#c12 .

[David Roche, 2013-03-22 19:17:11]

https://codereview.chromium.org/13008005/diff/1/chrome/browser/chromeos/prefe...
File chrome/browser/chromeos/preferences.cc (right):

https://codereview.chromium.org/13008005/diff/1/chrome/browser/chromeos/prefe...
chrome/browser/chromeos/preferences.cc:239:
registry->RegisterIntegerPref(prefs::kLanguageRemapCapsLockKeyTo,
On 2013/03/22 18:42:21, Yusuke Sato wrote:
> I think we need to rename kLanguageRemapCapsLockKeyTo in
> chrome/common/pref_names.cc and
> chrome/browser/resources/options/chromeos/keyboard_overlay.html to
> "settings.language.remap_caps_lock_key_to_r2" to avoid the scenario I wrote in
> https://code.google.com/p/chromium/issues/detail?id=167237#c12 .

Is changing the pref name really required, since it's no longer sync'd?  Any
build that contains the change to event_rewriter.cc will also contain this
change to avoid sync'ing this pref.

I based this change on one you did a while ago to turn off sync'ing for similar
reasons, and in that CL you only set the pref to UNSYNCHABLE_PREF but left the
pref name as-is.  https://codereview.chromium.org/7013051/

[Yusuke Sato, 2013-03-23 00:12:08]

> Is changing the pref name really required, since it's no longer sync'd?  

Since we want to remove the current Caps Lock setting that has already sync'd to
Chromebook.

> Any build that contains the change to event_rewriter.cc will also contain this
> change to avoid sync'ing this pref.

no.

> I based this change on one you did a while ago to turn off sync'ing for
similar
> reasons, and in that CL you only set the pref to UNSYNCHABLE_PREF but left the
> pref name as-is.  https://codereview.chromium.org/7013051/

In this case, I believe changing the name is required because we want to make
the pref UNSYNCABLE _and_ want to blow off the current users' Caps Lock setting.
Otherwise, for example, in
https://code.google.com/p/chromium/issues/detail?id=167237#c12 case, Caps Lock
on the external USB keyboard connected to Pixel will be still Ctrl, and the user
does not have an easy way to change it.

[David Roche, 2013-03-25 19:41:33]

On 2013/03/23 00:12:08, Yusuke Sato wrote:
> > Is changing the pref name really required, since it's no longer sync'd?  
> 
> Since we want to remove the current Caps Lock setting that has already sync'd
to
> Chromebook.
> 
> > Any build that contains the change to event_rewriter.cc will also contain
this
> > change to avoid sync'ing this pref.
> 
> no.
> 
> > I based this change on one you did a while ago to turn off sync'ing for
> similar
> > reasons, and in that CL you only set the pref to UNSYNCHABLE_PREF but left
the
> > pref name as-is.  https://codereview.chromium.org/7013051/
> 
> In this case, I believe changing the name is required because we want to make
> the pref UNSYNCABLE _and_ want to blow off the current users' Caps Lock
setting.
> Otherwise, for example, in
> https://code.google.com/p/chromium/issues/detail?id=167237#c12 case, Caps Lock
> on the external USB keyboard connected to Pixel will be still Ctrl, and the
user
> does not have an easy way to change it.

I see, you want to reset any previously-sync'd values to the default.  This does
avoid the issue you mentioned in 167237#c12, but it also has the side-effect of
removing the users preference setting where is is visible and valid ... i.e.,
all Chromebox values will be reset to the default.  Is this okay?

[David Roche, 2013-04-02 00:27:31]

Renamed pref as requested, PTAL.

[Yusuke Sato, 2013-04-02 04:27:18]

whoa, forgot to send LGTM... sorry. Patch set #2 LGTM.

Please ask derat@ for owners review. Please also get a green light from Alex in
167237.

[Kuscher, 2013-04-04 13:51:53]

It seems bad to reset the pref for all users. 

Also, I am really hoping we will work on the rest of the solution from the bug.
Enabling CapsLock UI for all external keyboard seems really what we want. At
that point in can also be synced again.

On 2013/03/25 19:41:33, David Roche wrote:
> On 2013/03/23 00:12:08, Yusuke Sato wrote:
> > > Is changing the pref name really required, since it's no longer sync'd?  
> > 
> > Since we want to remove the current Caps Lock setting that has already
sync'd
> to
> > Chromebook.
> > 
> > > Any build that contains the change to event_rewriter.cc will also contain
> this
> > > change to avoid sync'ing this pref.
> > 
> > no.
> > 
> > > I based this change on one you did a while ago to turn off sync'ing for
> > similar
> > > reasons, and in that CL you only set the pref to UNSYNCHABLE_PREF but left
> the
> > > pref name as-is.  https://codereview.chromium.org/7013051/
> > 
> > In this case, I believe changing the name is required because we want to
make
> > the pref UNSYNCABLE _and_ want to blow off the current users' Caps Lock
> setting.
> > Otherwise, for example, in
> > https://code.google.com/p/chromium/issues/detail?id=167237#c12 case, Caps
Lock
> > on the external USB keyboard connected to Pixel will be still Ctrl, and the
> user
> > does not have an easy way to change it.
> 
> I see, you want to reset any previously-sync'd values to the default.  This
does
> avoid the issue you mentioned in 167237#c12, but it also has the side-effect
of
> removing the users preference setting where is is visible and valid ... i.e.,
> all Chromebox values will be reset to the default.  Is this okay?

[David Roche, 2013-04-04 14:26:12]

Does it really make sense for this to be a sync'able pref?  That doesn't seem
like the desired behavior, since each device could have a different keyboard
layout attached to it (different language layouts, with vastly different uses
for the modifier keys).  It seems to me that these sorts of hardware-specific
settings should be local to the current hardware.

[James Cook, 2013-05-06 21:16:45]

LGTM

[James Cook, 2013-05-06 21:53:01]

On 2013/05/06 21:16:45, James Cook (Chromium) wrote:
> LGTM

Actually, not LGTM. I agree with kuscher - don't reset the pref for existing
users.  I think just keeping the existing pref name should be fine.

[David Roche, 2013-05-07 00:25:27]

Okay, I reverted the change to the pref name, which removed 
chrome/browser/resources/options/chromeos/keyboard_overlay.html and
chrome/common/pref_names.cc from the CL.

(Note that due to patch set 2 being messed up, the "Delta from patch set" links
are messed up, but the side-by-side diffs against base are fine.)

[James Cook, 2013-05-07 00:29:34]

LGTM. Thanks for the patch!

[David Roche, 2013-05-07 00:40:26]

+zel for chrome/browser/chromeos/preferences.cc approval

[zel, 2013-05-07 00:42:37]

lgtm

[commit-bot: I haz the power, 2013-05-07 21:24:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidroche@chromium.org/13008005/38001

[commit-bot: I haz the power, 2013-05-08 21:12:21]

Change committed as 198999