320429 - Switching tabs in a tabbox can result in illogical assignment of focus

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Tim Martin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

If the current tab does not have focus, selecting another causes the next item in the tabring to gain focus.  If the new tab doesn't contain any focusable elements, focus will be assigned to an item outside the tabbox.  This can even cause the location bar to gain focus, which is especially illogical (and causes additional problems; see Bug 320427).

Note that, in the test case, switching from tab B to tab A will cause the textbox in tabpanel A to gain focus, which probably makes sense.  But because tab B doesn't contain such an element, Firefox just focuses on the next available item outside of the tabbox, which can surprise the user.

If, when switching tabs, the current tab element has focus, then focus is transferred to the new tab, and that should probably be the behavior when there's no focusable element in a tab.

Reproducible: Always

Steps to Reproduce:
1. Open the testcase XUL file
2. Focus on something which is not tab A (such as the textbox)
3. Click on another tab

Actual Results:  
The next item after the tabbox gains focus.

Expected Results:  
If a tab contains no elements which can gain focus, the tab element should probably get focus.

Comment 1

[Steve England [:stevee]]

Attachment: testcase from reporter's URL

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Caused by the call to advanceFocusIntoSubtree.

Comment 3

[Nickolay_Ponomarev]

Attachment: make tabbox check that the focus doesn't leave it

Comment 4

[[e-mail]]

Comment on attachment 206814 [details] [diff] [review]
make tabbox check that the focus doesn't leave it

We could really do with an "advanceFocusWithinSubtree" method...

>+            // Make sure that the focus doesn't move outside the tabbox
>+            var parent = this.parentNode;
>+            while (parent && parent.localName != "tabbox")
>+              parent = parent.parentNode;
Nit: I don't think we want to do the rest of the tests if no tabbox is found, compare with the selectedIndex setter.

>+            var el = document.commandDispatcher.focusedElement;
Nit: This could throw an exception in content, so you ought to try/catch it.

Comment 5

[Nickolay_Ponomarev]

Attachment: update to comments

The original code seemed simpler to me...

Comment 6

[[e-mail]]

Comment on attachment 208597 [details] [diff] [review]
update to comments

Sorry for the delay.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/content/widgets/tabbox.xml; new revision: 1.32;
mozilla/xpfe/global/resources/content/bindings/tabbox.xml; new revision: 1.39;

Comment 8

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/content/widgets/tabbox.xml 			1.28.2.3
mozilla/xpfe/global/resources/content/bindings/tabbox.xml 	1.35.8.4