Remove "Go Back/Forward" from keyboard overlay

Remove "Go Back/Forward" from keyboard overlay

Only show these shortcuts if the finch experiment that turns them on is
enabled.

BUG=633825
TEST=manually
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[afakhry, 2016-08-03 22:16:47]

Description was changed from

==========
Remove "Go Back/Forward" from keyboard overlay

Only show these shortcuts if the finch experiment that turns them on is
enabled.

BUG=633825
TEST=manually
==========

to

==========
Remove "Go Back/Forward" from keyboard overlay

Only show these shortcuts if the finch experiment that turns them on is
enabled.

BUG=633825
TEST=manually
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[afakhry, 2016-08-03 22:19:46]

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

[afakhry, 2016-08-03 22:19:47]

xiyuan@chromium.org: Please review changes in 
keyboard_overlay*

thestig@chromium.org: Please review changes in 
The rest of files.

[afakhry, 2016-08-03 22:19:57]

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

[commit-bot: I haz the power, 2016-08-03 22:20:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiyuan, 2016-08-03 22:26:10]

keyboard_overlay* lgtm

[Lei Zhang, 2016-08-03 23:03:43]

lgtm

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

https://codereview.chromium.org/2206823004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/keyboard_overlay.js:558: if (shortcutId &&
(shortcutId == 'keyboardOverlayGoBack' ||
I'm not JS expert, so take this with a grain of salt, but do you need the
"shortcutId &&" check?

https://codereview.chromium.org/2206823004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/keyboard_overlay.js:563: if
(!loadTimeData.getBoolean('backspaceGoesBackFeatureEnabled'))
Tack this on to the if statement above?

[commit-bot: I haz the power, 2016-08-04 00:06:22]

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

[commit-bot: I haz the power, 2016-08-04 00:06:23]

Dry run: This issue passed the CQ dry run.

[afakhry, 2016-08-04 16:44:59]

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

https://codereview.chromium.org/2206823004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/keyboard_overlay.js:558: if (shortcutId &&
(shortcutId == 'keyboardOverlayGoBack' ||
On 2016/08/03 23:03:43, Lei Zhang wrote:
> I'm not JS expert, so take this with a grain of salt, but do you need the
> "shortcutId &&" check?

I'm not a JS person either, but you were right, no need for that "shortcutId
&&". Done.

https://codereview.chromium.org/2206823004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/keyboard_overlay.js:563: if
(!loadTimeData.getBoolean('backspaceGoesBackFeatureEnabled'))
On 2016/08/03 23:03:43, Lei Zhang wrote:
> Tack this on to the if statement above?

Done.

[Lei Zhang, 2016-08-04 16:55:05]

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

https://codereview.chromium.org/2206823004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/keyboard_overlay.js:560:
!loadTimeData.getBoolean('backspaceGoesBackFeatureEnabled')) {
This is indented 1 space too far. If the first eval is shorter, you wouldn't
write:

if (foo ||
     bar) {}

[afakhry, 2016-08-04 18:04:30]

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

https://codereview.chromium.org/2206823004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/keyboard_overlay.js:560:
!loadTimeData.getBoolean('backspaceGoesBackFeatureEnabled')) {
On 2016/08/04 16:55:04, Lei Zhang wrote:
> This is indented 1 space too far. If the first eval is shorter, you wouldn't
> write:
> 
> if (foo ||
>      bar) {}

Done.

[Lei Zhang, 2016-08-04 18:09:37]

++lgtm

[afakhry, 2016-08-04 18:14:25]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2016-08-04 18:14:26]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2206823004/#ps40001
(title: "Fix indent")

[commit-bot: I haz the power, 2016-08-04 18:14:49]

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

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-04 21:09:46]

Description was changed from

==========
Remove "Go Back/Forward" from keyboard overlay

Only show these shortcuts if the finch experiment that turns them on is
enabled.

BUG=633825
TEST=manually
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove "Go Back/Forward" from keyboard overlay

Only show these shortcuts if the finch experiment that turns them on is
enabled.

BUG=633825
TEST=manually
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/abbfa3e92777e0b55429cac3c880f12237732385
Cr-Commit-Position: refs/heads/master@{#409900}
==========

[commit-bot: I haz the power, 2016-08-04 21:09:47]

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