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

RESOLVED FIXED in mozilla10

Status

()

Core
Disability Access APIs
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Tom Brunet, Assigned: Neil Deakin (not available until Aug 9))

Tracking

(Blocks: 2 bugs, {access})

Trunk
mozilla10
x86
Windows XP
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Comment 1

11 years ago
Created attachment 255112 [details]
focusbug.xul - Original test case
(Reporter)

Comment 2

11 years ago
Created attachment 255113 [details]
focusbug2.xul - Two tabbox's to demonstrate additional oddities
(Reporter)

Comment 3

11 years ago
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.

Updated

11 years ago
Blocks: 83552
Keywords: access

Comment 4

11 years ago
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.
Assignee: aaronleventhal → oliver_yeoh

Comment 5

11 years ago
Confirmed bug on latest minefield.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

6 years ago
Can you please check against Firefox 4?

Comment 7

6 years ago
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?
Blocks: 375743
Created attachment 561573 [details] [diff] [review]
Fix to focus the tab if an element in a different tabpanel is focused
Assignee: oliver_yeoh → enndeakin
Status: NEW → ASSIGNED
Attachment #561573 - Flags: review?(neil)

Comment 9

6 years ago
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?
Created attachment 561782 [details] [diff] [review]
Simpler patch
Attachment #561573 - Attachment is obsolete: true
Attachment #561782 - Flags: review?(neil)
Attachment #561573 - Flags: review?(neil)
Created attachment 561841 [details] [diff] [review]
Possible patch

This is what I was thinking of... (-w diff)
Attachment #561841 - Flags: feedback?(enndeakin)
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.
Created attachment 562062 [details] [diff] [review]
Patch as suggested. Also added a test for an element outside a tabpanel and fixed the title.
Attachment #561782 - Attachment is obsolete: true
Attachment #562062 - Flags: review?(neil)
Attachment #561782 - Flags: review?(neil)
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.
Attachment #562062 - Flags: review?(neil) → review+

Updated

6 years ago
Status: ASSIGNED → NEW
Flags: in-testsuite?

Updated

6 years ago
Status: NEW → ASSIGNED
Attachment #561841 - Flags: feedback?(enndeakin)
https://hg.mozilla.org/mozilla-central/rev/8d2df2739eaf
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10

Comment 16

6 years ago
Created attachment 563663 [details] [diff] [review]
a11y mochitest
Attachment #563663 - Flags: review?(marco.zehe)
Comment on attachment 563663 [details] [diff] [review]
a11y mochitest

r=me, thanks!
Attachment #563663 - Flags: review?(marco.zehe) → review+
Flags: in-testsuite? → in-testsuite+

Comment 18

6 years ago
inbound land of a11y test https://hg.mozilla.org/integration/mozilla-inbound/rev/11d110b351c5
https://hg.mozilla.org/mozilla-central/rev/11d110b351c5
You need to log in before you can comment on or make changes to this bug.