Bug 7138 – Implement tabindex for all elements, enabling accessible web apps

• Component: Accessibility
• Status: RESOLVED FIXED

[Aaron Leventhal]

Support tabindex for all elements, thus enabling accessible DHTML/AJAX web applications. Both IE and Gecko-based browsers such as Firefox now support tabIndex on all elements. The Web Applications spec from WHATWG also enables tabIndex for all elements (http://whatwg.org/specs/web-apps/current-work/#the-tabindex)

Basically, any element such as a div or span can be added to the default tab order by setting tabindex="0". In addition, any item with tabindex="-1" can be focused via script or mouse. 

The tabindex system is needed for developing custom widgets which are accessible, from simple widgets like a slider to container widgets like a menubar, tree view or spreadsheet. IBM needs this for web apps it is developing, in order for them to run properly on Safari. Other large organizations are also picking up DHTML accessibility.

* The Gecko documentation on what the behavior should be is here: http://www.mozilla.org/access/keyboard/tabindex
* The MSDN docs for IE's tabindex behavior is here: http://msdn.microsoft.com/workshop/author/dhtml/reference/properties/tabindex.asp
* Test cases and more docs about DHTML accessibility here: http://www.mozilla.org/access/dhtml
* W3C standards work in this area: http://www.w3.org/WAI/PF/roadmap/DHTMLRoadmap110505.html

Work we had to do in Gecko (see https://bugzilla.mozilla.org/show_bug.cgi?id=171366):
- Have each element class support an IsFocusable method which also returns a boolean out to indicate whether it is tabbable.
  Specific implementations of the method need to upcall to the generic element's method, which jas the generic tabindex implementation..
  Then the scripting, mouse click handling and tab key handling  code needs to use the new IsFocusable() method.
- Support the focus() method in nsIDOMNSHTMLElement, which is outside of the official DOM interface but is supported fro all elements.
- Add rules to the default style sheet so that any element with :focus receives a focus outline
- Make cancelling an keydown event cancel the keypress event, for purposes of compatibility with IE. Authors writing cross-browser content should use keydown events to process keystrokes, because IE won't fire keypress events for non-alphanumeric keys.
- Ensure that all elements which are focusable are exposed in the accessibility API hierarchy, so that when they get focused there is an object to fire an accessibility event for.
- Any element that can receive focus must fire onfocus/onblur

[Joost de Valk (AlthA)]

Seems a reasonable request, confirming.

[Mark Malone]

rdar://4911289

[David Kilzer (:ddkilzer)]

See also Bug 9460.

[Timothy Hatcher]

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.

[Nicholas Shanks]

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.

[Adam Roben (:aroben)]

(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.

[Aaron Leventhal]

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.

[David Bolter]

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).

[David Kilzer (:ddkilzer)]

(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.

[Jesse Costello-Good]

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.

[David Kilzer (:ddkilzer)]

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

[Victor Tsaran]

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.

[Aaron Leventhal]

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.

[Aaron Leventhal]

Note: Opera, IE and Firefox all have this capability now.

Also, the bug for actual ARIA support is bug 12132.

[John A. Bilicki III]

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.

[Alexey Proskuryakov]

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

[Aaron Leventhal]

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.

[Robert Blaut]

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

[Johan Lund]

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.

[Johan Lund]

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

[Brian Moeskau]

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

[Sebastian Werner]

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

[Alice Liu]

Created attachment 20746 [details]
patch and regression test

[Adam Roben (:aroben)]

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.

[Sjoerd Visscher]

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.

[Adam Roben (:aroben)]

(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.

[Sjoerd Visscher]

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.

[Aaron Leventhal]

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.

[Aaron Leventhal]

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.

[Anne van Kesteren]

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).

[Darin Adler]

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.

[Maciej Stachowiak]

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

[Adam Roben (:aroben)]

(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

[Ian 'Hixie' Hickson]

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

[Sjoerd Visscher]

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 −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.

[Anne van Kesteren]

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

[Anne van Kesteren]

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. 

[Alice Liu]

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

[Darin Adler]

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