DevTools: fix deepActiveElement and focus on Dialog when document blurs

DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902
Committed: https://crrev.com/6ad9d6295756dbc10cbfa1c4563f1dd477343b15
Cr-Commit-Position: refs/heads/master@{#403337}

[luoe, 2016-06-23 23:01:38]

Description was changed from

==========
DevTools: give default focus elements to custom settings tabs

BUG=614902
==========

to

==========
DevTools: give default focus elements to custom settings tabs

The Devices and Throttling tabs in the settings screen do not extend TabbedPane,
so they lack a default focused element. When selected, the currentFocusElement
is null, preventing use of the 'Esc' to close the dialog because the key handler
doesn't have focus.

BUG=614902
==========

[luoe, 2016-06-23 23:01:53]

luoe@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-06-24 17:53:51]

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/NetworkConditionsSelector.js
(right):

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/NetworkConditionsSelector.js:287:
this.setDefaultFocusedElement(addButton);
How does this call interact with new override?

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/emulation/DevicesSettingsTab.js
(right):

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DevicesSettingsTab.js:35:
this.setDefaultFocusedElement(this._addCustomButton);
ditto

[luoe, 2016-06-24 20:41:53]

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/NetworkConditionsSelector.js
(right):

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/NetworkConditionsSelector.js:287:
this.setDefaultFocusedElement(addButton);
On 2016/06/24 17:53:51, dgozman wrote:
> How does this call interact with new override?

Good comment, I think this can be safely removed.

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/emulation/DevicesSettingsTab.js
(right):

https://codereview.chromium.org/2094873002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DevicesSettingsTab.js:35:
this.setDefaultFocusedElement(this._addCustomButton);
On 2016/06/24 17:53:51, dgozman wrote:
> ditto

This could be safely removed as well.

[luoe, 2016-06-29 21:22:03]

Description was changed from

==========
DevTools: give default focus elements to custom settings tabs

The Devices and Throttling tabs in the settings screen do not extend TabbedPane,
so they lack a default focused element. When selected, the currentFocusElement
is null, preventing use of the 'Esc' to close the dialog because the key handler
doesn't have focus.

BUG=614902
==========

to

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

The old deepActiveElement() returned the deepest shadow root's activeElement,
while this CL makes it return the deepest activeElement (which may not be in the
deepest shadow root).  Clicking to focus the Devices and Throttling tabs in the
settings screen will no longer trigger a blur, so pressing 'Esc' should work.

If the document blurs while a dialog is open, the dialog will take focus, so
that 'Esc' can always be handled.

BUG=614902
==========

[luoe, 2016-06-29 21:24:25]

Description was changed from

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

The old deepActiveElement() returned the deepest shadow root's activeElement,
while this CL makes it return the deepest activeElement (which may not be in the
deepest shadow root).  Clicking to focus the Devices and Throttling tabs in the
settings screen will no longer trigger a blur, so pressing 'Esc' should work.

If the document blurs while a dialog is open, the dialog will take focus, so
that 'Esc' can always be handled.

BUG=614902
==========

to

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

The old deepActiveElement() returned the deepest shadow root's activeElement,
while this CL makes it return the deepest activeElement (which may not be in the
deepest shadow root).  Opening the Devices and Throttling tabs in the
settings screen will no longer trigger a blur, so pressing 'Esc' should work.
This CL also sets a default button for the Blackbox settings tab.

If the document blurs while a dialog is open, the dialog will take focus, so
that 'Esc' can always be handled.

BUG=614902
==========

[luoe, 2016-06-29 21:37:56]

This CL has been changed so that the default buttons on Devices, Throttling,
Blackboxing settings tabs work.  PTAL

[dgozman, 2016-06-29 22:19:15]

https://codereview.chromium.org/2094873002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2094873002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:881:
WebInspector.Dialog.focus();
I wonder how the dialog is different from document? Why do be respect the blur
on document, but not dialog?

[luoe, 2016-06-30 00:03:16]

I've updated the last patch to have Dialogs put their keydown handler on the
document body.

https://codereview.chromium.org/2094873002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2094873002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:881:
WebInspector.Dialog.focus();
On 2016/06/29 22:19:15, dgozman wrote:
> I wonder how the dialog is different from document? Why do be respect the blur
> on document, but not dialog?

When the document is blurred (e.g. clicking on a non-focusable element),
document.activeElement can be == document.body.  So if the body has focus when a
dialog is open and 'Esc' is pressed, Dialog's onKeyDown won't fire and it won't
close.

I don't have a great reason why this is the right way to make sure 'Esc' always
works.  Maybe a better fix is to have Dialogs add an event listener to the
ownerDocument.body, just like how StylesSidebarPane does it.

[dgozman, 2016-06-30 17:06:55]

This looks pretty good. A couple of questions:
- do "add custom XXX" buttons get a default focus now?
- does Esc work when adding a new device (it should cancel the edit box)?
- does Esc open/close drawer when you click around the dialog (it shouldn't)?
- please update patch description according to the changes.

https://codereview.chromium.org/2094873002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/Dialog.js (right):

https://codereview.chromium.org/2094873002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/Dialog.js:60:
WebInspector.Dialog.focus = function()
Unused.

[luoe, 2016-06-30 17:36:10]

Description was changed from

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

The old deepActiveElement() returned the deepest shadow root's activeElement,
while this CL makes it return the deepest activeElement (which may not be in the
deepest shadow root).  Opening the Devices and Throttling tabs in the
settings screen will no longer trigger a blur, so pressing 'Esc' should work.
This CL also sets a default button for the Blackbox settings tab.

If the document blurs while a dialog is open, the dialog will take focus, so
that 'Esc' can always be handled.

BUG=614902
==========

to

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902
==========

[luoe, 2016-06-30 17:48:05]

Yes to all.

- 'Add custom...' buttons have default focus
- pressing Esc once after clicking 'Add custom...' just cancels the new entry
- pressing Esc a second time closes the entire settings dialog
- Open/close drawer does not seem affected
- description updated

https://codereview.chromium.org/2094873002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/Dialog.js (right):

https://codereview.chromium.org/2094873002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/Dialog.js:60:
WebInspector.Dialog.focus = function()
On 2016/06/30 17:06:55, dgozman wrote:
> Unused.

Done.

[dgozman, 2016-06-30 17:53:23]

Thanks! lgtm

[luoe, 2016-06-30 17:54:12]

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

[commit-bot: I haz the power, 2016-06-30 17:54:33]

Dry run: 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-06-30 18:39:16]

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

[commit-bot: I haz the power, 2016-06-30 18:39:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[luoe, 2016-06-30 19:05:32]

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

[commit-bot: I haz the power, 2016-06-30 19:06:36]

Dry run: 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-06-30 21:10:02]

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

[commit-bot: I haz the power, 2016-06-30 21:10:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[luoe, 2016-06-30 21:19:51]

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

[commit-bot: I haz the power, 2016-06-30 21:20:17]

Dry run: 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-06-30 22:09:11]

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

[commit-bot: I haz the power, 2016-06-30 22:09:12]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-06-30 23:23:01]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2016-06-30 23:23:50]

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-06-30 23:33:41]

Description was changed from

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902
==========

to

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902
==========

[commit-bot: I haz the power, 2016-06-30 23:33:42]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-30 23:33:44]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 23:34:50]

Description was changed from

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902
==========

to

==========
DevTools: fix deepActiveElement and focus on Dialog when document blurs

- Before, deepActiveElement() returned the deepest shadow root's
activeElement. Now, it returns the deepest activeElement (which may not be in
the deepest shadow root)
- Buttons 'Add pattern', 'Add custom device', 'Add custom profile' in the
Blackboxing, Devices, and Throttling settings tabs now have default focus
- Now that Dialog's keydown handler is on the body, pressing Esc now closes the
settings dialog, even after the document blurs

BUG=614902

Committed: https://crrev.com/6ad9d6295756dbc10cbfa1c4563f1dd477343b15
Cr-Commit-Position: refs/heads/master@{#403337}
==========

[commit-bot: I haz the power, 2016-06-30 23:34:51]

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