370396 - Control+Tab to an empty tab panel in a tabbox causes focus to leave the tabbox

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Tom Brunet]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 (CK-IBM) Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070213 Minefield/3.0a3pre

Focus behavior is inconsistent when there are empty panels in a tabbox.

Reproducible: Always

Steps to Reproduce:
1. Open the .xul file attached (focusbug.xul).  This file has a tabbox with three tabpanels.  The first tabpanel has a series of checkboxes.  The other tabpanels only have a description element (nothing focusable).
2. Use tab or click to put focus onto any of the checkboxes (e.g. Monday)
3. Press Ctrl+Tab or Shift+Ctrl+Tab
Actual Results:  
After Ctrl+Tab of Shift+Ctrl+Tab is pressed, focus jumps to the address bar.

Expected Results:  
Expected to see focus appear on the tab of the second panel "Tab2".  This ensures that subsequent Ctrl+Tab and Shift+Control Tab presses make sense.

Comment 1

[Tom Brunet]

Attachment: focusbug.xul - Original test case

Comment 2

[Tom Brunet]

Attachment: focusbug2.xul - Two tabbox's to demonstrate additional oddities

Comment 3

[Tom Brunet]

Some additional notes:

If the search box is open, focus will jump there instead of the address bar.  To fully characterize where focus is jumping, I have attached a second .xul file, focusbug2.xul.  Here are some scenarios:

1) Focus Monday in the first set of checkboxes.  Press control+Tab three times to return to "Tab1".  (Note that focus has not immediately left the tabbox).  Press tab.  Focus jumps to Monday of the second set of checkboxes rather than to Monday of the first set of checkboxes.
2) Open "Tab2" of the second tabbox and repeat case 1.  Focus now jumps past the second tabbox - to the find box or address bar.
3) Perform case 1 on the second tabbox.  Focus now behaves like the original defect report (focus immediately jumps away rather than waiting for tab key to be pressed).

Note in cases 1&2 that it is not jumping to the next item in the tab order because that would be the tab of the tabbox, and not its contents.

Comment 4

[Aaron Leventhal]

I remember that a similar issue used to bite us in the page info dialog -- there was one panel with nothing focusable, but that changed and this issue still remains.

Comment 5

[Wayne DeAngelo]

Confirmed bug on latest minefield.

Comment 6

[alexander :surkov (:asurkov)]

Can you please check against Firefox 4?

Comment 7

[alexander :surkov (:asurkov)]

On trunk there's no DOM focus or blur events when switching to tab that doesn't have focusable content within tabpanel. I'd expect that tab should take a focus if tabpanel content can't accept it.

Enn, can you please take a look?

Comment 8

[Neil Deakin]

Attachment: Fix to focus the tab if an element in a different tabpanel is focused

Comment 9

[[e-mail]]

Comment on attachment 561573 [details] [diff] [review]
Fix to focus the tab if an element in a different tabpanel is focused

Am I overlooking something or can you not just test for the focus being contained within the panel as distinct from the tabbox?

Comment 10

[Neil Deakin]

Attachment: Simpler patch

Comment 11

[[e-mail]]

Attachment: Possible patch

This is what I was thinking of... (-w diff)

Comment 12

[[e-mail]]

Comment on attachment 561782 [details] [diff] [review]
Simpler patch

>                 let el = document.commandDispatcher.focusedElement;
>-                while (el && el != this.tabbox)
>+                while (el && el != this.tabbox && el != this.tabbox.tabpanels) {
>+                  if (el == selectedPanel)
>+                    return;
>                   el = el.parentNode;
>+                }
>                 if (el != this.tabbox)
>                   aNewTab.focus();
So, you confused me by introducing a new code path for the selectedPanel which had the same effect as the old code path for the tabbox, but for the tabpanels you simply reused the existing code path for a parent outside of the tabbox.

Probably the simplest code would result from early returning when either the tabbox or selectedPanel is reached, and exiting the loop when either the panels is reached or you run out of parents, and always focusing the new tab in that case.

The other alternative I would accept is to add the selected panel to both the loop condition and the focus condition instead of the early return.

Comment 13

[Neil Deakin]

Attachment: Patch as suggested. Also added a test for an element outside a tabpanel and fixed the title.

Comment 14

[[e-mail]]

Comment on attachment 562062 [details] [diff] [review]
Patch as suggested. Also added a test for an element outside a tabpanel and fixed the title.

Your tests still seems to pass with my patch for some reason.

Comment 15

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/8d2df2739eaf

Comment 16

[alexander :surkov (:asurkov)]

Attachment: a11y mochitest

Comment 17

[Marco Zehe (:MarcoZ)]

Comment on attachment 563663 [details] [diff] [review]
a11y mochitest

r=me, thanks!

Comment 18

[alexander :surkov (:asurkov)]

inbound land of a11y test https://hg.mozilla.org/integration/mozilla-inbound/rev/11d110b351c5

Comment 19

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/11d110b351c5