Bug 1506787: Support tabindex attribute (including value -1) on non-control XUL elements. r=smaug
authorJames Teh <jteh@mozilla.com>
Sat, 17 Nov 2018 02:38:27 +0000
changeset 446882 38844846c6ae6773afb8b9254bf3dbfec437d1ec
parent 446881 7be95c95a3a7254bcff022190ab2be27ca27ac50
child 446883 22e8321b69f180a1d74975fd8f162326a49d0efe
push id35052
push userapavel@mozilla.com
push dateSat, 17 Nov 2018 11:25:40 +0000
treeherdermozilla-central@efc1da42132b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1506787
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1506787: Support tabindex attribute (including value -1) on non-control XUL elements. r=smaug Previously, the tabindex attribute wasn't supported on non-control XUL elements at all. The only way to make those focusable was to use -moz-user-focus: normal. However, that caused the element to be included in the tab order; there was no way to make it focusable but not tabbable. This can now be achieved using tabindex="-1". This will primarily be useful for buttons on toolbars, which will be grouped under a single tab stop for efficiency. For consistency, this also changes the behaviour of tabindex="-1" with -moz-user-focus: ignore on XUL controls. Previously, -moz-user-focus: ignore would override tabindex="-1", making the element unfocusable. Now, the tabindex attribute always overrides if explicitly specified. Differential Revision: https://phabricator.services.mozilla.com/D12000
dom/interfaces/xul/nsIDOMXULControlElement.idl
dom/tests/mochitest/chrome/window_focus.xul
dom/webidl/XULElement.webidl
dom/xul/nsXULElement.cpp
toolkit/content/tests/chrome/test_tabindex.xul
--- a/dom/interfaces/xul/nsIDOMXULControlElement.idl
+++ b/dom/interfaces/xul/nsIDOMXULControlElement.idl
@@ -2,20 +2,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 interface nsIControllers;
 
-[scriptable, uuid(ea7f92d0-b379-4107-91b4-1e69bdd771e3)]
+[scriptable, uuid(bdc1d047-6d22-4813-bc50-638ccb349c7d)]
 interface nsIDOMXULControlElement : nsISupports {
   attribute boolean disabled;
-  attribute long tabIndex;
   
 // XXX defined in XULElement, but should be defined here
 //  readonly attribute nsIControllers controllers;
   
 //  void focus();
 //  void blur();
 };
 
--- a/dom/tests/mochitest/chrome/window_focus.xul
+++ b/dom/tests/mochitest/chrome/window_focus.xul
@@ -1632,17 +1632,17 @@ SimpleTest.waitForFocus(startTest);
   <button id="o1" accesskey="i" label="tabindex = -1" tabindex="-1"/>
   <richlistbox id="t5" label="tabindex = 0" tabindex="0" width="50">
     <richlistitem height="10"/>
   </richlistbox>
   <button id="t1" label="tabindex = 2" tabindex="2"/>
 </hbox>
 <hbox>
   <button id="o2" accesskey="o" style="-moz-user-focus: ignore;" label="no tabindex"/>
-  <button id="o4" style="-moz-user-focus: ignore;" label="tabindex = -1" tabindex="-1"/>
+  <button id="o4" style="-moz-user-focus: ignore;" label="no tabindex"/>
   <button id="t6" style="-moz-user-focus: ignore;" label="tabindex = 0" tabindex="0"/>
   <button id="t2" style="-moz-user-focus: ignore;" label="tabindex = 2" tabindex="2"/>
 </hbox>
 <hbox id="specialroot">
   <button id="t7" style="-moz-user-focus: normal;" label="no tabindex"/>
   <button id="o3" style="-moz-user-focus: normal;" label="tabindex = -1" tabindex="-1"/>
   <button id="t8" style="-moz-user-focus: normal;" label="tabindex = 0" tabindex="0"/>
   <richlistbox id="t3" style="-moz-user-focus: normal;" label="tabindex = 2" tabindex="2" width="50">
@@ -1658,27 +1658,27 @@ SimpleTest.waitForFocus(startTest);
   <button id="o22" label="tabindex = -1" tabindex="-1" disabled="true"/>
   <button id="o24" label="tabindex = 0" tabindex="0" disabled="true"/>
   <button id="o26" label="tabindex = 2" tabindex="2" disabled="true"/>
 </hbox>
 </vbox>
 <vbox>
 <hbox>
   <dropmarker id="o6" value="no tabindex"/>
-  <dropmarker id="o8" value="tabindex = -1" tabindex="-1"/>
-  <dropmarker id="o10" value="tabindex = 0" tabindex="0"/>
-  <dropmarker id="o12" value="tabindex = 2" tabindex="2"/>
+  <dropmarker id="o8" value="no tabindex"/>
+  <dropmarker id="o10" value="no tabindx"/>
+  <dropmarker id="o12" value="no tabindex"/>
   <dropmarker id="t9" accesskey="r" style="-moz-user-focus: normal;" value="no tabindex" />
-  <dropmarker id="t10" style="-moz-user-focus: normal;" value="tabindex = -1" tabindex="-1" />
+  <dropmarker id="t10" style="-moz-user-focus: normal;" value="no tabindex"/>
   <dropmarker id="t11" style="-moz-user-focus: normal;" value="tabindex = 0" tabindex="0" />
-  <dropmarker id="t12" style="-moz-user-focus: normal;" value="tabindex = 2" tabindex="2" />
+  <dropmarker id="t12" style="-moz-user-focus: normal;" value="no tabindex"/>
   <dropmarker id="o14" style="-moz-user-focus: ignore;" value="no tabindex"/>
-  <dropmarker id="o16" style="-moz-user-focus: ignore;" value="tabindex = -1" tabindex="-1"/>
-  <dropmarker id="n1" style="-moz-user-focus: none;" value="tabindex = 0" tabindex="0"/>
-  <dropmarker id="n3" style="-moz-user-focus: none;" value="tabindex = 2" tabindex="2"/>
+  <dropmarker id="o16" style="-moz-user-focus: ignore;" value="no tabindex"/>
+  <dropmarker id="n1" style="-moz-user-focus: none;" value="no tabindex"/>
+  <dropmarker id="n3" style="-moz-user-focus: none;" value="no tabindex"/>
 </hbox>
 </vbox>
 <browser id="childframe" type="content" src="child_focus_frame.html" width="300" height="195"/>
 <button id="t34"/>
 <tabbox id="tabbox">
   <tabs><tab id="t35" label="One"/><tab id="tab2" label="Two"/></tabs>
   <tabpanels>
     <tabpanel>
--- a/dom/webidl/XULElement.webidl
+++ b/dom/webidl/XULElement.webidl
@@ -70,16 +70,18 @@ interface XULElement : Element {
 
   attribute boolean allowEvents;
 
   [Throws, ChromeOnly]
   readonly attribute XULControllers             controllers;
   [Throws]
   readonly attribute BoxObject?                 boxObject;
 
+  [SetterThrows]
+  attribute long tabIndex;
   [Throws]
   void                      focus();
   [Throws]
   void                      blur();
   [NeedsCallerType]
   void                      click();
   void                      doCommand();
 
--- a/dom/xul/nsXULElement.cpp
+++ b/dom/xul/nsXULElement.cpp
@@ -434,26 +434,25 @@ nsXULElement::IsFocusableInternal(int32_
    *
    * Confusingly, the supplied value for the aTabIndex argument may indicate
    * whether the element may be focused as a result of the -moz-user-focus
    * property, where -1 means no and 0 means yes.
    *
    * For controls, the element cannot be focused and is not part of the tab
    * order if it is disabled.
    *
-   * Controls (those that implement nsIDOMXULControlElement):
+   * -moz-user-focus is overridden if a tabindex (even -1) is specified.
+   *
+   * Specifically, the behaviour for all XUL elements is as follows:
    *  *aTabIndex = -1  no tabindex     Not focusable or tabbable
-   *  *aTabIndex = -1  tabindex="-1"   Not focusable or tabbable
+   *  *aTabIndex = -1  tabindex="-1"   Focusable but not tabbable
    *  *aTabIndex = -1  tabindex=">=0"  Focusable and tabbable
    *  *aTabIndex >= 0  no tabindex     Focusable and tabbable
    *  *aTabIndex >= 0  tabindex="-1"   Focusable but not tabbable
    *  *aTabIndex >= 0  tabindex=">=0"  Focusable and tabbable
-   * Non-controls:
-   *  *aTabIndex = -1                  Not focusable or tabbable
-   *  *aTabIndex >= 0                  Focusable and tabbable
    *
    * If aTabIndex is null, then the tabindex is not computed, and
    * true is returned for non-disabled controls and false otherwise.
    */
 
   // elements are not focusable by default
   bool shouldFocus = false;
 
@@ -478,46 +477,42 @@ nsXULElement::IsFocusableInternal(int32_
       if (aTabIndex)
         *aTabIndex = -1;
       return false;
     }
     shouldFocus = true;
   }
 
   if (aTabIndex) {
-    if (xulControl) {
-      if (HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)) {
-        // if either the aTabIndex argument or a specified tabindex is non-negative,
-        // the element becomes focusable.
-        int32_t tabIndex = 0;
-        xulControl->GetTabIndex(&tabIndex);
-        shouldFocus = *aTabIndex >= 0 || tabIndex >= 0;
-        *aTabIndex = tabIndex;
-      } else {
-        // otherwise, if there is no tabindex attribute, just use the value of
-        // *aTabIndex to indicate focusability. Reset any supplied tabindex to 0.
-        shouldFocus = *aTabIndex >= 0;
-        if (shouldFocus)
-          *aTabIndex = 0;
+    if (HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)) {
+      // The tabindex attribute was specified, so the element becomes
+      // focusable.
+      shouldFocus = true;
+      *aTabIndex = TabIndex();
+    } else {
+      // otherwise, if there is no tabindex attribute, just use the value of
+      // *aTabIndex to indicate focusability. Reset any supplied tabindex to 0.
+      shouldFocus = *aTabIndex >= 0;
+      if (shouldFocus) {
+        *aTabIndex = 0;
       }
+    }
 
-      if (shouldFocus && sTabFocusModelAppliesToXUL &&
-          !(sTabFocusModel & eTabFocus_formElementsMask)) {
-        // By default, the tab focus model doesn't apply to xul element on any system but OS X.
-        // on OS X we're following it for UI elements (XUL) as sTabFocusModel is based on
-        // "Full Keyboard Access" system setting (see mac/nsILookAndFeel).
-        // both textboxes and list elements (i.e. trees and list) should always be focusable
-        // (textboxes are handled as html:input)
-        // For compatibility, we only do this for controls, otherwise elements like <browser>
-        // cannot take this focus.
-        if (IsNonList(mNodeInfo))
-          *aTabIndex = -1;
+    if (xulControl && shouldFocus && sTabFocusModelAppliesToXUL &&
+        !(sTabFocusModel & eTabFocus_formElementsMask)) {
+      // By default, the tab focus model doesn't apply to xul element on any system but OS X.
+      // on OS X we're following it for UI elements (XUL) as sTabFocusModel is based on
+      // "Full Keyboard Access" system setting (see mac/nsILookAndFeel).
+      // both textboxes and list elements (i.e. trees and list) should always be focusable
+      // (textboxes are handled as html:input)
+      // For compatibility, we only do this for controls, otherwise elements like <browser>
+      // cannot take this focus.
+      if (IsNonList(mNodeInfo)) {
+        *aTabIndex = -1;
       }
-    } else {
-      shouldFocus = *aTabIndex >= 0;
     }
   }
 
   return shouldFocus;
 }
 
 bool
 nsXULElement::HasMenu()
@@ -1030,16 +1025,21 @@ nsXULElement::RemoveTooltipSupport()
 
 bool
 nsXULElement::ParseAttribute(int32_t aNamespaceID,
                              nsAtom* aAttribute,
                              const nsAString& aValue,
                              nsIPrincipal* aMaybeScriptedPrincipal,
                              nsAttrValue& aResult)
 {
+    if (aNamespaceID == kNameSpaceID_None &&
+        aAttribute == nsGkAtoms::tabindex) {
+      return aResult.ParseIntValue(aValue);
+    }
+
     // Parse into a nsAttrValue
     if (!nsStyledElement::ParseAttribute(aNamespaceID, aAttribute, aValue,
                                          aMaybeScriptedPrincipal, aResult)) {
         // Fall back to parsing as atom for short values
         aResult.ParseStringOrAtom(aValue);
     }
 
     return true;
@@ -1955,16 +1955,20 @@ nsXULPrototypeElement::SetAttrAt(uint32_
           DeclarationBlock::FromCssText(
             aValue, data, eCompatibility_FullStandards, nullptr);
         if (declaration) {
             mAttributes[aPos].mValue.SetTo(declaration.forget(), &aValue);
 
             return NS_OK;
         }
         // Don't abort if parsing failed, it could just be malformed css.
+    } else if (mAttributes[aPos].mName.Equals(nsGkAtoms::tabindex)) {
+        mAttributes[aPos].mValue.ParseIntValue(aValue);
+
+        return NS_OK;
     }
 
     mAttributes[aPos].mValue.ParseStringOrAtom(aValue);
 
     return NS_OK;
 }
 
 void
--- a/toolkit/content/tests/chrome/test_tabindex.xul
+++ b/toolkit/content/tests/chrome/test_tabindex.xul
@@ -8,111 +8,136 @@
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>  
   <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>  
 
 <!--
   Elements are navigated in the following order:
     1. tabindex > 0 in tree order
     2. tabindex = 0 in tree order
-  Elements with tabindex = -1 are not in the tab order
+  Elements with tabindex = -1 are focusable, but not in the tab order
  -->
 <hbox>
-  <button id="t5" label="One"/>
-  <checkbox id="no1" label="Two" tabindex="-1"/>
-  <button id="t6" label="Three" tabindex="0"/>
+  <button id="t7" label="One"/>
+  <checkbox id="f1" label="Two" tabindex="-1"/>
+  <button id="t8" label="Three" tabindex="0"/>
   <checkbox id="t1" label="Four" tabindex="1"/>
 </hbox>
 <hbox>
-  <textbox id="t7" idmod="t3" size="3"/>
-  <textbox id="no2" size="3" tabindex="-1"/>
-  <textbox id="t8" idmod="t4" size="3" tabindex="0"/>
+  <textbox id="t9" idmod="t5" size="3"/>
+  <textbox id="f2" size="3" tabindex="-1"/>
+  <textbox id="t10" idmod="t6" size="3" tabindex="0"/>
   <textbox id="t2" idmod="t1" size="3" tabindex="1"/>
 </hbox>
 <hbox>
-  <button id="no3" style="-moz-user-focus: ignore;" label="One"/>
-  <checkbox id="no4" style="-moz-user-focus: ignore;" label="Two" tabindex="-1"/>
-  <button id="t9" style="-moz-user-focus: ignore;" label="Three" tabindex="0"/>
+  <button id="n1" style="-moz-user-focus: ignore;" label="One"/>
+  <checkbox id="f3" style="-moz-user-focus: ignore;" label="Two" tabindex="-1"/>
+  <button id="t11" style="-moz-user-focus: ignore;" label="Three" tabindex="0"/>
   <checkbox id="t3" style="-moz-user-focus: ignore;" label="Four" tabindex="1"/>
 </hbox>
 <hbox>
-  <textbox id="t10" idmod="t5" style="-moz-user-focus: ignore;" size="3"/>
-  <textbox id="no5" style="-moz-user-focus: ignore;" size="3" tabindex="-1"/>
-  <textbox id="t11" idmod="t6" style="-moz-user-focus: ignore;" size="3" tabindex="0"/>
+  <textbox id="t12" idmod="t7" style="-moz-user-focus: ignore;" size="3"/>
+  <textbox id="f4" style="-moz-user-focus: ignore;" size="3" tabindex="-1"/>
+  <textbox id="t13" idmod="t8" style="-moz-user-focus: ignore;" size="3" tabindex="0"/>
   <textbox id="t4" idmod="t2" style="-moz-user-focus: ignore;" size="3" tabindex="1"/>
 </hbox>
-<richlistbox id="t12" idmod="t7">
+<richlistbox id="t14" idmod="t9">
   <richlistitem><label value="Item One"/></richlistitem>
 </richlistbox>
 
 <hbox>
-  <!-- the tabindex attribute does not apply to non-controls, so it
-       should be treated as -1 for non-focusable dropmarkers, and 0
-       for focusable dropmarkers. Thus, the first four dropmarkers
-       are not in the tab order, and the last four dropmarkers should
-       be in the tab order just after the listbox above.
+  <!-- the tabindex attribute applies to non-controls as well. They are not
+       focusable unless tabindex is explicitly specified.
     -->
-  <dropmarker id="no6"/>
-  <dropmarker id="no7" tabindex="-1"/>
-  <dropmarker id="no8" tabindex="0"/>
-  <dropmarker id="no9" tabindex="1"/>
-  <dropmarker id="t13" style="-moz-user-focus: normal;"/>
-  <dropmarker id="t14" style="-moz-user-focus: normal;" tabindex="-1"/>
-  <dropmarker id="t15" style="-moz-user-focus: normal;" tabindex="0"/>
-  <dropmarker id="t16" style="-moz-user-focus: normal;" tabindex="1"/>
+  <dropmarker id="n2"/>
+  <dropmarker id="f5" tabindex="-1"/>
+  <dropmarker id="t15" tabindex="0"/>
+  <dropmarker id="t5" idmod="t3" tabindex="1"/>
+  <dropmarker id="t16" style="-moz-user-focus: normal;"/>
+  <dropmarker id="f6" style="-moz-user-focus: normal;" tabindex="-1"/>
+  <dropmarker id="t17" style="-moz-user-focus: normal;" tabindex="0"/>
+  <dropmarker id="t6" idmod="t4" style="-moz-user-focus: normal;" tabindex="1"/>
 </hbox>
 
 <body xmlns="http://www.w3.org/1999/xhtml">
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
 <pre id="test">
 </pre>
 </body>
 
 <script>
 <![CDATA[
 
 SimpleTest.waitForExplicitFinish();
 
+function checkFocusability(aId, aFocusable)
+{
+  document.activeElement.blur();
+  let testNode = document.getElementById(aId);
+  testNode.focus();
+  let newFocus = document.activeElement;
+  if (newFocus.localName == "input") {
+    newFocus = document.getBindingParent(newFocus);
+  }
+  let check = aFocusable ? is : isnot;
+  let focusableText = aFocusable ? "focusable " : "unfocusable ";
+  check(newFocus, testNode,
+    ".focus() call on " + focusableText + aId);
+}
+
 var gAdjustedTabFocusModel = false;
-var gTestCount = 16;
+var gTestCount = 17;
 var gTestsOccurred = 0;
- 
+let gFocusableNotTabbableCount = 6;
+let gNotFocusableCount = 2;
+
 function runTests()
 {
   var t;
-  window.addEventListener("focus", function (event) {
+  function onFocus(event) {
     if (t == 1 && event.target.id == "t2") {
       // looks to be using the MacOSX Full Keyboard Access set to Textboxes
       // and lists only so use the idmod attribute instead
       gAdjustedTabFocusModel = true;
-      gTestCount = 7;
+      gTestCount = 9;
     }
 
     var attrcompare = gAdjustedTabFocusModel ? "idmod" : "id";
 
     // check for the last test which should wrap aorund to the first item
     // consider the focus event on the inner input of textboxes instead
     if (event.originalTarget.localName == "input") {
       is(document.getBindingParent(event.originalTarget).getAttribute(attrcompare),
          "t" + t, "tab " + t + " to inner input");
       gTestsOccurred++;
     }
     else {
       is(event.target.getAttribute(attrcompare), "t" + t, "tab " + t + " to " + event.target.localName)
       if (event.target.localName != "textbox")
         gTestsOccurred++;
     }
-  }, true);
+  }
+  window.addEventListener("focus", onFocus, true);
 
   for (t = 1; t <= gTestCount; t++)
     synthesizeKey("KEY_Tab");
 
   is(gTestsOccurred, gTestCount, "test count");
+  window.removeEventListener("focus", onFocus, true);
+
+  for (let i = 1; i <= gFocusableNotTabbableCount; ++i) {
+    checkFocusability("f" + i, true);
+  }
+
+  for (let i = 1; i <= gNotFocusableCount; ++i) {
+    checkFocusability("n" + i, false);
+  }
+
   SimpleTest.finish();
 }
 
 SimpleTest.waitForFocus(runTests);
 
 ]]>
 
 </script>