Bug 7138 - Implement tabindex for all elements, enabling accessible web apps
: Implement tabindex for all elements, enabling accessible web apps
Status: RESOLVED FIXED
: WebKit
Accessibility
: 420+
: All All
: P2 Normal
Assigned To:
: http://developer.mozilla.org/en/docs/...
: InRadar, ReviewedForRadar
:
: 4986 10489 12132 12728 13846
  Show dependency treegraph
 
Reported: 2006-02-07 18:45 PST by
Modified: 2009-01-12 15:24 PST (History)


Attachments
patch and regression test (80.36 KB, patch)
2008-04-22 02:37 PST, Alice Liu
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2006-02-07 18:45:50 PST
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
------- Comment #1 From 2006-02-15 14:16:39 PST -------
Seems a reasonable request, confirming.
------- Comment #2 From 2007-01-05 14:34:50 PST -------
rdar://4911289
------- Comment #3 From 2007-01-07 05:04:22 PST -------
See also Bug 9460.
------- Comment #4 From 2007-04-02 11:58:55 PST -------
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.
------- Comment #5 From 2007-05-15 16:30:03 PST -------
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.
------- Comment #6 From 2007-05-15 17:00:47 PST -------
(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.
------- Comment #7 From 2007-05-16 06:34:32 PST -------
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.
------- Comment #8 From 2007-05-23 09:06:14 PST -------
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).
------- Comment #9 From 2007-05-23 10:33:40 PST -------
(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.
------- Comment #10 From 2007-06-28 10:05:34 PST -------
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.
------- Comment #11 From 2007-07-05 21:58:52 PST -------
*** Bug 13464 has been marked as a duplicate of this bug. ***
------- Comment #12 From 2007-08-01 14:21:20 PST -------
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.
------- Comment #13 From 2007-09-25 18:34:07 PST -------
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.
------- Comment #14 From 2007-09-25 18:41:17 PST -------
Note: Opera, IE and Firefox all have this capability now.

Also, the bug for actual ARIA support is bug 12132.
------- Comment #15 From 2007-10-25 12:06:27 PST -------
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.
------- Comment #16 From 2008-01-31 02:55:58 PST -------
*** Bug 17114 has been marked as a duplicate of this bug. ***
------- Comment #17 From 2008-03-19 17:13:08 PST -------
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.
------- Comment #18 From 2008-03-21 11:51:25 PST -------
*** Bug 17983 has been marked as a duplicate of this bug. ***
------- Comment #19 From 2008-03-26 03:08:55 PST -------
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.
------- Comment #20 From 2008-03-26 08:37:13 PST -------
For Bindows support we will be needing at least focusable elements of type div and tr.
------- Comment #21 From 2008-04-05 14:56:43 PST -------
Add another vote for fixing this from the Ext JS team.  Thanks!
------- Comment #22 From 2008-04-11 14:58:01 PST -------
We (the qooxdoo team) have worked around the thing, but it would still be great to have native support for this in webkit.
------- Comment #23 From 2008-04-22 02:37:18 PST -------
Created an attachment (id=20746) [details]
patch and regression test
------- Comment #24 From 2008-04-22 05:44:20 PST -------
(From update of attachment 20746 [details])
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.
------- Comment #25 From 2008-04-22 06:23:15 PST -------
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.
------- Comment #26 From 2008-04-22 06:36:24 PST -------
(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.
------- Comment #27 From 2008-04-22 06:40:29 PST -------
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.
------- Comment #28 From 2008-04-22 06:48:31 PST -------
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.
------- Comment #29 From 2008-04-22 06:52:49 PST -------
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.
------- Comment #30 From 2008-04-22 07:14:25 PST -------
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 #31 From 2008-04-22 09:44:40 PST -------
(From update of attachment 20746 [details])
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.
------- Comment #32 From 2008-04-22 11:49:23 PST -------
I'm not sure fast/forms makes sense, since tabIndex allows things that are not form elements to be focued too.
------- Comment #33 From 2008-04-23 09:51:03 PST -------
(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
------- Comment #34 From 2008-04-24 02:53:47 PST -------
I've revamped the HTML5 spec. Let me know if I missed anything. It should be much more precise about what should happen now.
------- Comment #35 From 2008-04-24 03:23:00 PST -------
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.
------- Comment #36 From 2008-04-24 06:47:36 PST -------
FWIW: IE returns 0 for such elements (attribute absent, attribute error, or not focusable by default) and Firefox and Opera -1.
------- Comment #37 From 2008-04-24 06:48:30 PST -------
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. 
------- Comment #38 From 2008-04-28 18:33:11 PST -------
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
------- Comment #39 From 2009-01-12 15:24:19 PST -------
*** Bug 20596 has been marked as a duplicate of this bug. ***