1022726 - Toolbox keyboard navigation is broken when switched from docked to detached.

Status: RESOLVED FIXED

(DevTools :: General, defect, P1)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Seen on latest nightly and current Aurora

STR:

1. open toolbox in any docking mode (  bottom, side, detached )
2. click to select the inspector, then click to select any node in the current page to ensure keyboard focus is on the toolbox ( check by using the keyboard to navigate to the console and back again )
3. move the toolbox to a different docking mode
4. try to use the keyboard to navigate to a different tool pane

Expected: 

 - should be able to switch immediately

Actual

 - no keybindings seem to work at all

Workaround

 - clicking the mouse on anything in the toolbox restores keyboard focus.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Just tested Aurora and Stable, they suffer the same issue. Chrome works fine.

Comment 2

[Brian Grinstead [:bgrins]]

Bumped into this on Bug 1119133 as well

Comment 3

[Jeff Griffiths (:canuckistani) (:⚡︎)]

I've always thought this was pretty bad, P1.

Comment 4

[Patrick Brosset <:pbro>]

doing 'this.doc.defaultView.focus();' in Toolbox.prototype.switchHost, after the new host got created works, sometimes.
It fixes it when switching from a window host to a side or bottom host, and back, but the problem remains when switching from a side host to a bottom host, and back. And I can't figure out how to re-assign the focus. It seems lost somehow, and the only way to make it come back is to press tab, which focuses the address bar in the browser tab.

Comment 5

[Patrick Brosset <:pbro>]

Attachment: bug1022726.diff

I have no idea why, but this works on my mac.
Brian, did you investigate this issue while working on bug 1119133? If you have any idea why simply focusing the window once doesn't work when going from side to bottom, I'm all ears.

Comment 6

[Brian Grinstead [:bgrins]]

I never figured it out.  I suspected it had something to do with the call to swapFrameLoaders though.  Your patch actually works though!  Tested on osx and windows.  So nice job figuring it out :)

Comment 7

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: bz://1022726/pbrosset

/r/9051 - Bug 1022726 - Focus the devtools window after switching host type

Pull down this commit:

hg pull -r 934ac81b6b37bd8ab5526477556f7693a958adcb https://reviewboard-hg.mozilla.org/gecko/

Comment 8

[Patrick Brosset <:pbro>]

mozreview shows one of the tests as deleted/created, but this was just a 'hg mv'

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8607544 [details]
MozReview Request: bz://1022726/pbrosset

https://reviewboard.mozilla.org/r/9049/#review7651

Ship It!

Comment 10

[Patrick Brosset <:pbro>]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c25685a2f0a

Comment 11

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Would love this for 40.1.

Comment 12

[Patrick Brosset <:pbro>]

(In reply to Jeff Griffiths (:canuckistani) from comment #11)
> Would love this for 40.1.
Ok, so the patch is R+, which is a start, but it looks like a few tests are failing with it. So I hope I can fix this quickly and in on time for 40.1.

Comment 13

[Patrick Brosset <:pbro>]

New try push with a correction I made to the test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5464d4c9643d

Comment 14

[Patrick Brosset <:pbro>]

https://hg.mozilla.org/integration/fx-team/rev/e261c91790f3

Comment 15

[Patrick Brosset <:pbro>]

Let's see if that sticks and lands in m-c. If yes and we still have time, I'll request aurora-uplift so we get it in 40.

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e261c91790f3

Comment 17

[Patrick Brosset <:pbro>]

Attachment: bug1022726-focus-toolbox.patch

This is the patch that finally landed (after I made the last test modifications).
Asking for aurora-uplift as requested by Jeff.

Approval Request Comment
[Feature/regressing bug #]: This isn't related to a recent half-finished feature nor a regression. This is just something that wasn't implemented a long time ago.
[User impact if declined]: If declined, the user experience using the devtools is impacted: when you switch from a host type to another host type (from bottom docked to a detached window for example), the keyboard shortcuts don't work straight away, you first need to click in the toolbox to give it focus, which defeats the whole purpose of using the keyboard.
[Describe test coverage new/current, TreeHerder]: Has been on m-c for a few days, including a new mochitest.
[Risks and why]: The code change in this patch is 2 lines of window focusing code when switching devtools host, so there is no risk for the browser itself. The only (minor) risk is for devtools users to switch host, if these 2 lines misbehave, but the test I added hasn't failed since it landed.
[String/UUID change made/needed]: None.

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8610062 [details] [diff] [review]
bug1022726-focus-toolbox.patch

If it is a long standing bug, it is early in the aurora cycle, it has tests, so, taking it.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b08fab71b4f2

Comment 21

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: Bug 1022726 - Focus the devtools window after switching host type