Deprecate Shift+Alt to switch IME and use Ctrl+Shift+Space instead.

Deprecate Shift+Alt to switch IME and use Ctrl+Shift+Space instead.

Shift+Alt can conflict with many other accelerators and can easily
be triggered by mistake when the user doesn't mean it. We changed it
to Ctrl+Shift+Space to be similar to the Ctrl+Space for PreviousIME.

BUG=535004
TEST=ash_unittests --gtest_filter=DeprecatedAcceleratorTester.*

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

[afakhry, 2015-11-05 19:37:40]

Description was changed from

==========
Deprecate Shift+Alt to switch IME and use Ctrl+Shift+Space instead.

BUG=535004
TEST=ash_unittests --gtest_filter=DeprecatedAcceleratorTester.*
==========

to

==========
Deprecate Shift+Alt to switch IME and use Ctrl+Shift+Space instead.

Shift+Alt can conflict with many other accelerators and can easily
be triggered by mistake when the user doesn't mean it. We changed it
to Ctrl+Shift+Space to be similar to the Ctrl+Space for PreviousIME.

BUG=535004
TEST=ash_unittests --gtest_filter=DeprecatedAcceleratorTester.*
==========

[afakhry, 2015-11-05 19:37:40]

afakhry@chromium.org changed reviewers:
+ oshima@chromium.org

[afakhry, 2015-11-05 19:38:08]

Oshima, Can you please review? Thanks!

[afakhry, 2015-11-05 19:40:01]

afakhry@chromium.org changed reviewers:
+ xiyuan@chromium.org

[afakhry, 2015-11-05 19:40:01]

Xiyuan, Can you please review keyboard_overlay_data.js? Thanks!

[xiyuan, 2015-11-05 20:33:21]

https://codereview.chromium.org/1414483011/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/keyboard_overlay_data.js (right):

https://codereview.chromium.org/1414483011/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/keyboard_overlay_data.js:15817:
'space<>CTRL<>SHIFT': 'keyboardOverlayCycleThroughInputMethods',
nit: please keep the list alpha sorted.

[afakhry, 2015-11-05 20:46:42]

https://codereview.chromium.org/1414483011/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/keyboard_overlay_data.js (right):

https://codereview.chromium.org/1414483011/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/keyboard_overlay_data.js:15817:
'space<>CTRL<>SHIFT': 'keyboardOverlayCycleThroughInputMethods',
On 2015/11/05 20:33:21, xiyuan wrote:
> nit: please keep the list alpha sorted.

Done.

[xiyuan, 2015-11-05 20:51:40]

keyboard_overlay_data.js LGTM

Thanks.

[afakhry, 2015-11-05 20:53:02]

afakhry@chromium.org changed reviewers:
+ mpearson@chromium.org

[afakhry, 2015-11-05 20:53:03]

+mpearson@ for histograms.xml. Thanks!

[oshima, 2015-11-05 21:12:18]

lgtm

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
File ash/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
ash/accelerators/accelerator_table.cc:102: // Ctrl+Shift+Space.
crbug.com?

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
ash/accelerators/accelerator_table.cc:207: // 5- Don't forget to update the
keyboard overlay.
add description to chrome/browser/resources/chromeos/keyboard_overlay_data.js

[Mark P, 2015-11-05 21:45:24]

histograms.xml lgtm

[afakhry, 2015-11-05 22:13:36]

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
File ash/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
ash/accelerators/accelerator_table.cc:102: // Ctrl+Shift+Space.
On 2015/11/05 21:12:18, oshima wrote:
> crbug.com?

Done.

https://codereview.chromium.org/1414483011/diff/40001/ash/accelerators/accele...
ash/accelerators/accelerator_table.cc:207: // 5- Don't forget to update the
keyboard overlay.
On 2015/11/05 21:12:18, oshima wrote:
> add description to chrome/browser/resources/chromeos/keyboard_overlay_data.js

Done.

[afakhry, 2015-11-09 17:52:17]

Oshima, following the UX request to try to keep shortcut keys text (e.g.
Ctrl+Shift+Space) in the notification message on one line, please take a look at
the changes introduced in this patch.

[oshima, 2015-11-09 19:49:28]

Alternative way is to construct shortcut key string from table but uses
translated key names like IDS_MODIFIER_SHIFT, IDS_MODIFIER_SEARCH etc (if they
needs to be translated). This way, you can avoid adding IDS string for each
shortcuts deprecated.

https://codereview.chromium.org/1414483011/diff/120001/ash/accelerators/accel...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/1414483011/diff/120001/ash/accelerators/accel...
ash/accelerators/accelerator_controller.cc:119: // Ensures that there are no
word breaks at the "+"s in the shortcut texts such
what if the shortcut contains "+" key itself?

[afakhry, 2015-11-10 00:56:47]

> Alternative way is to construct shortcut key string from table but uses
> translated key names like IDS_MODIFIER_SHIFT, IDS_MODIFIER_SEARCH etc (if they
> needs to be translated). This way, you can avoid adding IDS string for each
> shortcuts deprecated.

Thank you for your suggestion! As per our discussion, some languages translate
these keys and some don't. We're not sure whether it's a language based decision
or depending on the context. It is also going to be hard to predict the
translation results of individual keys in non-US keyboard layouts. We can leave
it as is right now, and see if we need to modify this later.

https://codereview.chromium.org/1414483011/diff/120001/ash/accelerators/accel...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/1414483011/diff/120001/ash/accelerators/accel...
ash/accelerators/accelerator_controller.cc:119: // Ensures that there are no
word breaks at the "+"s in the shortcut texts such
On 2015/11/09 19:49:28, oshima wrote:
> what if the shortcut contains "+" key itself?

The "+" will just be appended and prepended with the word joiner character, so I
think it's probably going to be fine.

[afakhry, 2015-11-10 01:55:23]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2015-11-10 01:55:23]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
oshima@chromium.org, mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1414483011/#ps120001
(title: "ensure no line breaks in the middle of shortcuts texts")

[commit-bot: I haz the power, 2015-11-10 01:56:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1414483011/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1414483011/120001

[commit-bot: I haz the power, 2015-11-10 02:42:41]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-10 02:44:09]

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