1014758 - Can't switch away from a preferences tab using keyboard commands (Ctrl+Tab or Ctrl+PageUp/Down), if the preferences tab is showing "Advanced" and you've interacted with it

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Open a few tabs. Note that you can cycle through them via Ctrl+PageUp/Down, and Ctrl+Tab. (on Linux at least; maybe differs per-platform)

 2. Open Firefox Preferences. (spawns a tab)

 3. Click "Advanced", and then interact with the Advanced prefs' panel somehow. (e.g. click any checkbox (twice if you don't want to change its value), or click any of the General|Data Choices|etc. sub-tabs.)

 4. Now, try to switch away from this tab using the keyboard commands from step 1.

 5. Now, click another tab to switch to it, and try to cycle through your tabs using the keyboard commands from step 1.


ACTUAL RESULTS:
 - In step 4, I'm unable to switch away from the Preferences tab; it seems to co-opt the tab-switching key combinations for navigating between its own "General|Data Choices|etc." sub-tabs

 - In step 5, I'm unable to cycle through my tabs; as soon as I hit the Preferences tab, it absorbs all subsequent tab-switching key combinations.

Comment 1

[Daniel Holbert [:dholbert]]

My version info:
 Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0
 32.0a1 (2014-05-22)

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Reproduced this on Windows8.1, switching to All.

This appears to be the default behavior of <xul:tabpanels>. The solution that I see here is to stop using a tab panel, as its use also hurts our ability to do in-content search results where we may want to show contents of multiple <xul:tabpanel>s simultaneously.

Fang, should we block shipping the in-content preferences until we have a new design that doesn't use these tabpanels? Can you work with Michael to come up with the design?

Comment 3

[Zhenshuo Fang (:fang) - Firefox UX Team]

Yes I think eventually we want to have a new design for "Advanced" that doesn't use tabs, mostly because we want to implement search, I filed bug 1017053 for this. If I remember correctly the searchable prefs work is not a blocker for shipping in-content prefs. 

For this particular issue, is there a work around or any way to change the behavior of <xul:tabpanels> while we are waiting on the new design? I think step 4 actually make sense, when you start interacting with the tabs in Advanced, we cycle through them. But in Step 5, we should cycle through only tabs, not Tabs + Tabs in Advanced panel.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Zhenshuo Fang (:fang) - Firefox UX Team from comment #3)
> For this particular issue, is there a work around or any way to change the
> behavior of <xul:tabpanels> while we are waiting on the new design?

From skimming http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/tabbox.xml (which IIUC is the implementation of tabpanels), it looks like we might be able to set...
  handleCtrlTab="false"
...on the <tabbox> element. We do this in a few other places, as shown by a MXR search for handleCtrlTab .

I suspect we'd also want handleCtrlPageUpDown="false", though we don't seem to use that in existing code, for some reason. (not sure why)

> I think
> step 4 actually make sense, when you start interacting with the tabs in
> Advanced, we cycle through them.

I can see that, though I think it makes just as much sense for the key-combinations to keep their standard behavior.  (And if the only fix we have for Step 5 would also affect Step 4, then I'd advocate proceeding with such a fix.)

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Richard, would you like to put together a patch that fixes this? You can follow the tips that Daniel put in comment #4.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: noCtrlTab.patch

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8430701 [details] [diff] [review]
noCtrlTab.patch

Did you try using handleCtrlPageUpDown="false" ? What did you find?

Comment 8

[Richard Marti (:Paenglab)]

Attachment: noCtrlTab.patch

Never used Ctrl+PgUp/PgDn but now it's also blocked.

Comment 9

[Daniel Holbert [:dholbert]]

Per https://groups.google.com/d/msg/mozilla.dev.platform/9w0-Bh_3vVI/xWebYK64GdwJ , the sheriffs (who land checkin-needed patches) are now requiring that checkin-needed bugs have a Try run, with whatever tests/platforms the patch-author deems appropriate.  Hence, canceling checkin-needed for the moment.

Richard: if you have TryServer access, could you give this a Try run? (I'm not sure what tests exercise this UI; maybe jaws can suggest the right testsuite(s) to run?)  Or if you don't have access, maybe jaws can do that on your behalf?

Comment 10

[Richard Marti (:Paenglab)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=6d5924b6f3b6

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7a9c57eb4564

Comment 13

[Camelia Badau [:cbadau], Desktop QA]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.8.5 using latest Aurora 32.0a2 (buildID: 20140610004002) and latest Nightly 33.0a1 (buildID: 20140610030202).