Bug 7138

Seems a reasonable request, confirming.

See also Bug 9460.

This would help the Web Inspector, since I am converting more of it to HTML. Right now I am working around this with a JavaScript solution.

There is a school of thought that considers tabindex harmful because it changes the users expectations: e.g. <input type=text tabindex=1 /> <input type=text tabindex=2 /> <input type=text tabindex=4 /> <input type=text tabindex=3 /> obviously real-world examples are (usually) more complex, but doing something other than what the user expects from a system UI she is used to is a bad thing.

(In reply to comment #5) > There is a school of thought that considers tabindex harmful because it changes > the users expectations: > > e.g. > > <input type=text tabindex=1 /> > <input type=text tabindex=2 /> > <input type=text tabindex=4 /> > <input type=text tabindex=3 /> > > obviously real-world examples are (usually) more complex, but doing something > other than what the user expects from a system UI she is used to is a bad > thing. > I would hope that web authors would be using tabindex to make the tab order _more_ intuitive and/or closer to the system behavior, not less so.

Adam, check out how tabindex="-1" and tabindex="0" are used to make container widgets that work like their system counterparts. From the sounds of it you're thinking about reordering the tab navigation which isn't what this is really about.

I think the severity of this is arguable much greater than "enhancement". We (at dojotoolkit) need this if we are to fully support (i.e. provide keyboard control) WebKit for our DHTML widgets (dijits).

(In reply to comment #8) > I think the severity of this is arguable much greater than "enhancement". We > (at dojotoolkit) need this if we are to fully support (i.e. provide keyboard > control) WebKit for our DHTML widgets (dijits). Bug 13846 was filed to track issues for the Dojo Toolkit.

We at TIBCO would very much like this to make it in to Safari ASAP. It is common in Ajax to make custom GUI controls that don't use any of the native browser INPUT tags. In order to make these Ajax controls seems like native controls we need to be able to focus SPAN and DIV.

*** Bug 13464 has been marked as a duplicate of this bug. ***

TabIndexability would be an important addition for sophari-based browsers, like the one used by Nokia on S60FP1 devices. Taming this basic issue would hopefully prevent DHTML apps on mobilephone to get out of hand by providing developers with a possibility of making these applications accessible.

Need testcases? Simple test cases from Opera: http://tc.labs.opera.com/html/global-attributes/tabindex/ Advanced test cases from Mozilla: http://www.mozilla.org/access/dhtml/checkbox http://www.mozilla.org/access/dhtml/tree http://www.mozilla.org/access/dhtml/spreadsheet http://www.mozilla.org/access/dhtml/pretty-slider Should also test against Dojo toolkit, which uses this extensively.

Note: Opera, IE and Firefox all have this capability now. Also, the bug for actual ARIA support is bug 12132.

A live test case, http://www.jabcreations.net/ which provides both a complex layout and accessibility. Firefox, Opera, and IE are all accessible on my site.

*** Bug 17114 has been marked as a duplicate of this bug. ***

I have some notes on how to implement this here: http://developer.mozilla.org/en/docs/ARIA_User_Agent_Implementors_Guide#11.3.1.1_HTML_5_Tabindex If you bug Simon Pieters (zcorpan) from Opera he will give you links to some simple test cases.

*** Bug 17983 has been marked as a duplicate of this bug. ***

The Bindows AJAX framework will not be able to fully support webkit until this has been fixed. Our custom focusing system requires it. Please implement this functionality as soon as possible.

For Bindows support we will be needing at least focusable elements of type div and tr.

Add another vote for fixing this from the Ext JS team. Thanks!

We (the qooxdoo team) have worked around the thing, but it would still be great to have native support for this in webkit.

Created attachment 20746 [details] patch and regression test

Comment on attachment 20746 [details] patch and regression test Is it OK that we're changing public OS X headers? Is it OK that we're removing headers that projects using WebKit are likely currently using? + bool m_hasChangedTabIndex : 1; Does Node have room for this extra bit? There might be a better name for this member, but I'm having trouble thinking of it. Names I've considered include m_hasExplicitTabIndex, m_didSetTabIndex, m_hasTabIndex. +++ WebCore/html/HTMLElement.idl (working copy) @@ -35,6 +35,10 @@ module html { attribute [ConvertNullToNullString] DOMString dir; attribute [ConvertNullToNullString] DOMString className; + attribute long tabIndex; + void blur(); + void focus(); + Indentation looks funny here but it probably is just matching the funny indentation of the rest of the file. +++ WebCore/html/HTMLLegendElement.cpp (working copy) @@ -45,7 +45,7 @@ HTMLLegendElement::~HTMLLegendElement() bool HTMLLegendElement::isFocusable() const { - return false; + return m_hasChangedTabIndex; } Should this behavior be in HTMLElement instead? It would be good to test Shift-Tab in the regression test as well. Your test doesn't seem to test calling focus() on any elements. I think this is a good thing to test, since focus() working on elements that are not normally focusable is the main effect of your patch! I'm not going to r+ or r- this yet, because I'm interested in the answers to my questions.

Hmm, there seems to be an incompatibility between IE and Firefox with tabIndex="-1". In IE, elements that are not focusable are also not focusable when tabIndex="-1". In Firefox they are focusable, and this seems what you have implemented here. But the HTML 5 spec seems to be compatible with IE.

(In reply to comment #25) > Hmm, there seems to be an incompatibility between IE and Firefox with > tabIndex="-1". In IE, elements that are not focusable are also not focusable > when tabIndex="-1". In Firefox they are focusable, and this seems what you have > implemented here. When you say "focusable", do you mean that they are included in the tab order (i.e., you can press Tab to reach them), or that you can call element.focus() on them and they will gain keyboard focus, or both, or something different entirely? > But the HTML 5 spec seems to be compatible with IE.

With focusable I mean that both .focus() and clicking on the element draws focus outlines on the element. I did not test if that also means that they receive keyboard events, but I would think so. I have also raised this issue on the whatwg list.

Putting tabindex="-1" on normally unfocusable items should make them focusable via click or script, but not tabbable. However, IE has what I consider a bug in that those items receive focus but don't provide a default focus appearance for them.

The WhatWG spec appears to be wrong, in that it says: > A negative integer specifies that the element should be removed from the tab > order. If the element does normally take focus, it may still be focused using > other means (e.g. it could be focused by a click). It should also say that a negative integer makes elements focusable but not tabbable, whether they are normally focusable or not. IE certainly does that -- they just don't show focus, which is really a section 508 mistake on their part. For IE the author has to put in an onfocus handler and set element.style to show focus.

Yes, aaronlev is right. Though theres a difference between IE and Firefox on the handling of invalid values, such as tabindex=blah or tabindex=1s. HTML5 follows IE here ("ignore" the attribute).

Comment on attachment 20746 [details] patch and regression test Really great work! I'm happy to see this getting fixed and I love having the new thorough test. + * bindings/objc/PublicDOMInterfaces.h: + -Remove focus, blur, tabindex from descendants of DOMHTMLElement. This comment confused me. You're not removing these -- you're moving them to HTMLElement. I think the name m_hasChangedTabIndex could be improved. I suggest m_tabIndexSetExplicitly, or m_hasTabIndex or m_tabIndexWasSet or m_tabIndexWasSetExplicitly. WebCore::Document uses m_titleSetExplicitly for a similar situation. bool m_focused : 1; + bool m_hasChangedTabIndex : 1; bool m_active : 1; You didn't change the comment below that says "1 bit left"; I think you took the last bit, so it has to say "no bits left". - setTabIndex(max(static_cast<int>(std::numeric_limits<short>::min()), min(indexstring.toInt(), static_cast<int>(std::numeric_limits<short>::max())))); + Node::setTabIndex(max(static_cast<int>(std::numeric_limits<short>::min()), min(indexstring.toInt(), static_cast<int>(std::numeric_limits<short>::max())))); I'm not happy with the idiom used here. I think the internal Node::setTabIndex function should be renamed to get it out of the way of the public HTMLElement::setTabIndex function. It's usually not good style to hide one non-virtual function with another that does something different. Where else is Node::setTabIndex called? Maybe we can make it be a protected function member? - return isContentEditable() && parent() && !parent()->isContentEditable(); + return (m_hasChangedTabIndex || (isContentEditable() && parent() && !parent()->isContentEditable())); Another way to write this would be to call Element::isFocusable() instead of checking the m_hasChangedTabIndex flag directly. I'd like to see that flag be private if possible (see below). No need for that outer set of parentheses around the entire return expression. Please omit them. bool HTMLFieldSetElement::isFocusable() const { - return false; + return m_hasChangedTabIndex; } Another way to do this is to call Element::isFocusable, since we're trying to undo what HTMLElement and HTMLGenericFormElement do. I think that's better than having a copy of the logic from Node::isFocusable here, but you might not agree. Same comment about HTMLLabelElement, HTMLLegendElement, HTMLOptGroupElement, and HTMLOptionElement. You might be able to make the flag private rather than protected if you eliminate these. For future maintenance it would be much better if the tabIndex-related data members were private. We may want to change them around without changing all the code. +<!--imbedded elements--> I think these are "embedded" as opposed to "imbedded". + printToConsole(elem.id + " blured"); Typo here: "blured" instead of "blurred" with two "r"s. + document.write(focusCount+" focus / "+blurCount+" blur events disatched<br>"+consoleOutput.innerHTML); Typo here: "disatched" instead of "dispatched". +<frame src="data:text/html,<div id='theConsole'>Console:</div>"/> I don't think the text "Console:" makes the output easier to read. I think this test should go in the fast/forms directory rather than fast/events -- even though the test involves the focus and blur events, I think it's really about the form element support for being focused. The test checks that elements can be focused, but I don't see tests to check each element's behavior with no tabIndex, tabIndex of -1, tabIndex of 0, and a positive tabIndex. The test checks the behavior of the tab key, but tabIndex has other effects as well. For example, it controls whether focus() works at all on some elements. We need tests to cover these changes in behavior. I'm interested in having tests to check that we have reasonable behavior when a tabIndex is set to values like: -2, -32767, -32768, -32769, 32767, 32768, and also large integers at the 2^31 and 2^32 boundaries. I worry that we truncate integers and do unpredictable things in those cases. I'd like the test output to be a little clearer about whether it's a success or failure. Currently the output doesn't make it clear that it's success. Also, the test doesn't seem to check the differences in behaviors. For example, where's the test for the difference between tabIndex of 0 and tabIndex of -1? r=me, as-is, but I'd like to see some refinements too.

I'm not sure fast/forms makes sense, since tabIndex allows things that are not form elements to be focued too.

(In reply to comment #27) > With focusable I mean that both .focus() and clicking on the element draws > focus outlines on the element. I did not test if that also means that they > receive keyboard events, but I would think so. > > I have also raised this issue on the whatwg list. Here's the link to the email thread. There's some interesting discussion in there: http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2008-April/014512.html

I've revamped the HTML5 spec. Let me know if I missed anything. It should be much more precise about what should happen now.

Just one thing: "The tabIndex DOM attribute must reflect the value of the tabIndex content attribute. If the attribute is not present, or parsing its value returns an error, then the DOM attribute must return 0 for elements that are focusable and &#8722;1 for elements that are not focusable." This does not make sense. A tabIndex="-1" means that the element is focusable, so returning -1 for the DOM attribute for unfocusable elements is wrong. The only value that makes sense is null.

FWIW: IE returns 0 for such elements (attribute absent, attribute error, or not focusable by default) and Firefox and Opera -1.

I should have added, if you want to determine whether an element is focuable having an additional getter would probably be good because contenteditable and maybe other attributes can influence that as well.

I re-vamped the test to have much better output, and to test .focus instead of tabbing, the former of which this patch is really about. Also addressed all of Darin's comments, some of which overlapped with Adam's comments. fast/dom/tabindex-clamp already addresses Darin's desire for a boundary condition on assignment test. I also addressed the recently added specification that elements with tabindex unspecified or parsed unsuccessfully shall return 0 for normally focusable elements and -1 for all others. Previous return value was undefined for the latter. committed r32664

*** Bug 20596 has been marked as a duplicate of this bug. ***