MD Settings: Display > Overscan: Update text and fix focus

MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling blur() on the focused button, or simply clicking or tabbing away from the dialog) would cause the keyboard event to not get triggered. This change was introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of this.addEventListener.

BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/857d7966566652606eebaf44e27db67f36d3d0dc
Cr-Commit-Position: refs/heads/master@{#440910}

[stevenjb, 2016-12-20 22:15:31]

Description was changed from

==========
MD Settings: Display > Overscan: Update text

BUG=660597
==========

to

==========
MD Settings: Display > Overscan: Update text

BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2016-12-20 22:20:13]

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

[stevenjb, 2016-12-20 22:38:40]

Description was changed from

==========
MD Settings: Display > Overscan: Update text

BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling
blur() on the focused button, or simply clicking or tabbing away from the
dialog) would cause the keyboard event to not get triggered. This change was
introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of
this.addEventListener.


BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-12-22 07:02:26]

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:2664: <message
name="IDS_SETTINGS_DISPLAY_OVERSCAN_SUBTITLE" desc="Subtitle for the display
overscan settings subpate.">
subpate => subpage

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:2671: Shrink / Expand
source for this change? Since these spaces aren't standard English.

https://codereview.chromium.org/2589353002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display_overscan_dialog.js
(right):

https://codereview.chromium.org/2589353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display_overscan_dialog.js:36: //
changing focus does not prevent key events from occuring.
I don't understand the focus bug. Shouldn't tabbing away make the arrow keys
stop affecting the overscan position? Or is the overscan dialog "special"
because of the overlay thingy that happens on-screen?

[stevenjb, 2016-12-22 20:54:14]

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:2664: <message
name="IDS_SETTINGS_DISPLAY_OVERSCAN_SUBTITLE" desc="Subtitle for the display
overscan settings subpate.">
On 2016/12/22 07:02:26, michaelpg (OOO) wrote:
> subpate => subpage

Done.

https://codereview.chromium.org/2589353002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:2671: Shrink / Expand
On 2016/12/22 07:02:26, michaelpg (OOO) wrote:
> source for this change? Since these spaces aren't standard English.

From the issue:
https://storage.googleapis.com/monorail-prod.appspot.com/16/attachments/2aebc...

https://codereview.chromium.org/2589353002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display_overscan_dialog.js
(right):

https://codereview.chromium.org/2589353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display_overscan_dialog.js:36: //
changing focus does not prevent key events from occuring.
On 2016/12/22 07:02:26, michaelpg (OOO) wrote:
> I don't understand the focus bug. Shouldn't tabbing away make the arrow keys
> stop affecting the overscan position? Or is the overscan dialog "special"
> because of the overlay thingy that happens on-screen?

The ovescan dialog is "special" because of the overlay thingy that happens
on-screen.

[michaelpg, 2016-12-28 22:27:48]

lgtm

[stevenjb, 2016-12-28 23:26:11]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2016-12-28 23:26:20]

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-12-29 00:20:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482967571216390,
"parent_rev": "c3db2bb4e127b0a7e3d58c1ac38d0f1e73dda118", "commit_rev":
"f14b85fd001e27a62d38b86e4dabbbdbd70215a3"}

[commit-bot: I haz the power, 2016-12-29 00:21:19]

Description was changed from

==========
MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling
blur() on the focused button, or simply clicking or tabbing away from the
dialog) would cause the keyboard event to not get triggered. This change was
introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of
this.addEventListener.


BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling
blur() on the focused button, or simply clicking or tabbing away from the
dialog) would cause the keyboard event to not get triggered. This change was
introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of
this.addEventListener.


BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2589353002
==========

[commit-bot: I haz the power, 2016-12-29 00:21:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2017-01-02 15:50:54]

Description was changed from

==========
MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling
blur() on the focused button, or simply clicking or tabbing away from the
dialog) would cause the keyboard event to not get triggered. This change was
introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of
this.addEventListener.


BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2589353002
==========

to

==========
MD Settings: Display > Overscan: Update text and fix focus

While updating the text I discovered that changing the focus (e.g. by calling
blur() on the focused button, or simply clicking or tabbing away from the
dialog) would cause the keyboard event to not get triggered. This change was
introduced in https://codereview.chromium.org/2180823004.

The simple fix is to call window.addEventListener instead of
this.addEventListener.

BUG=660597
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/857d7966566652606eebaf44e27db67f36d3d0dc
Cr-Commit-Position: refs/heads/master@{#440910}
==========

[commit-bot: I haz the power, 2017-01-02 15:50:55]

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