Title: [248914] trunk
Revision
248914
Author
rn...@webkit.org
Date
2019-08-20 12:59:00 -0700 (Tue, 20 Aug 2019)

Log Message

The default tab index of output and fieldset should be -1
https://bugs.webkit.org/show_bug.cgi?id=200834

Reviewed by Alex Christensen.

Source/WebCore:

This patch updates the default tab index of output and fieldset to -1 to match the behavior of
Chrome and Firefox as well as the latest HTML specification:
https://html.spec.whatwg.org/multipage/interaction.html#the-tabindex-attribute

To do that, this patch replaces HTMLFormControlElement::defaultTabIndex with defaultTabIndex
implementation in each one of its concrete subclass such as HTMLButtonElement to mirror
the language of the specification as follows:

HTMLFormControlElement
    HTMLButtonElement -> 0
    HTMLFieldSetElement -> -1
    HTMLFormControlElementWithState
        HTMLKeygenElement -> 0 - Not specified anywhere but preserving the existing behavior.
        HTMLSelectElement -> 0
        HTMLTextFormControlElement
            HTMLInputElement -> 0
            HTMLTextAreaElement -> 0
    HTMLOutputElement -> -1 

Even though Element::shouldBeIgnoredInSequentialFocusNavigation() checks the value of
defaultTabIndex, this patch does not affect the actual sequential (tab) focus navigation:
Beacuse HTMLOutputElement and HTMLFieldSetElement have supportsFocus overrides to call
HTMLElement::supportsFocus instead of HTMLFormControlElement::supportsFocus, these two
elements are focusable only when tabindex is set or it's a root content editable element.
But all root editable elements are focusable anyway and the default tab index does not
matter when tabindex is set.

Test: fast/dom/tabindex-defaults.html

* html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::defaultTabIndex const): Added.
* html/HTMLButtonElement.h:
* html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::defaultTabIndex const): Deleted.
* html/HTMLFormControlElement.h:
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::defaultTabIndex const): Added.
* html/HTMLKeygenElement.h:
* html/HTMLKeygenElement.cpp:
(WebCore::HTMLKeygenElement::defaultTabIndex const): Added.
* html/HTMLKeygenElement.h:
* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::defaultTabIndex const): Added.
* html/HTMLSelectElement.h:
* html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::defaultTabIndex const): Added.
* html/HTMLTextAreaElement.h:

LayoutTests:

Added test cases for output, fieldset, and keygen.

* fast/dom/tabindex-defaults-expected.txt:
* fast/dom/tabindex-defaults.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (248913 => 248914)


--- trunk/LayoutTests/ChangeLog	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/LayoutTests/ChangeLog	2019-08-20 19:59:00 UTC (rev 248914)
@@ -1,3 +1,15 @@
+2019-08-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        The default tab index of output and fieldset should be -1
+        https://bugs.webkit.org/show_bug.cgi?id=200834
+
+        Reviewed by Alex Christensen.
+
+        Added test cases for output, fieldset, and keygen.
+
+        * fast/dom/tabindex-defaults-expected.txt:
+        * fast/dom/tabindex-defaults.html:
+
 2019-08-20  Zalan Bujtas  <za...@apple.com>
 
         [ContentChangeObserver] isConsideredClickable should be able to process elements with no renderers

Modified: trunk/LayoutTests/fast/dom/tabindex-defaults-expected.txt (248913 => 248914)


--- trunk/LayoutTests/fast/dom/tabindex-defaults-expected.txt	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/LayoutTests/fast/dom/tabindex-defaults-expected.txt	2019-08-20 19:59:00 UTC (rev 248914)
@@ -8,22 +8,31 @@
 PASS input.tabIndex is 0
 PASS select.tabIndex is 0
 PASS textarea.tabIndex is 0
+PASS keygen.tabIndex is 0
 PASS editableDiv.tabIndex is 0
 PASS normalDiv.tabIndex is -1
+PASS output.tabIndex is -1
+PASS fieldset.tabIndex is -1
 PASS anchor.setAttribute("tabindex", "invalid"); anchor.tabIndex is 0
 PASS button.setAttribute("tabindex", "invalid"); button.tabIndex is 0
 PASS input.setAttribute("tabindex", "invalid"); input.tabIndex is 0
 PASS select.setAttribute("tabindex", "invalid"); select.tabIndex is 0
 PASS textarea.setAttribute("tabindex", "invalid"); textarea.tabIndex is 0
+PASS keygen.setAttribute("tabindex", "invalid"); keygen.tabIndex is 0
 PASS editableDiv.setAttribute("tabindex", "invalid"); editableDiv.tabIndex is 0
 PASS normalDiv.setAttribute("tabindex", "invalid"); normalDiv.tabIndex is -1
+PASS output.setAttribute("tabindex", "invalid"); output.tabIndex is -1
+PASS fieldset.setAttribute("tabindex", "invalid"); fieldset.tabIndex is -1
 PASS anchor.setAttribute("tabindex", "9999999999"); anchor.tabIndex is 0
 PASS button.setAttribute("tabindex", "9999999999"); button.tabIndex is 0
 PASS input.setAttribute("tabindex", "9999999999"); input.tabIndex is 0
 PASS select.setAttribute("tabindex", "9999999999"); select.tabIndex is 0
 PASS textarea.setAttribute("tabindex", "9999999999"); textarea.tabIndex is 0
+PASS keygen.setAttribute("tabindex", "9999999999"); keygen.tabIndex is 0
 PASS editableDiv.setAttribute("tabindex", "9999999999"); editableDiv.tabIndex is 0
 PASS normalDiv.setAttribute("tabindex", "9999999999"); normalDiv.tabIndex is -1
+PASS output.setAttribute("tabindex", "9999999999"); output.tabIndex is -1
+PASS fieldset.setAttribute("tabindex", "9999999999"); fieldset.tabIndex is -1
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/dom/tabindex-defaults.html (248913 => 248914)


--- trunk/LayoutTests/fast/dom/tabindex-defaults.html	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/LayoutTests/fast/dom/tabindex-defaults.html	2019-08-20 19:59:00 UTC (rev 248914)
@@ -11,10 +11,13 @@
 <input id="input">
 <select id="select"></select>
 <textarea id="textarea"></textarea>
+<keygen id="keygen"></keygen>
 <div id="editableDiv" contenteditable="true"></div>
 
 <!-- Unfocusable element -->
 <div id="normalDiv"></div>
+<output id="output"></output>
+<fieldset id="fieldset"></fieldset>
 </div>
 
 <script>
@@ -26,6 +29,9 @@
 var select = document.getElementById('select');
 var textarea = document.getElementById('textarea');
 var editableDiv = document.getElementById('editableDiv');
+var output = document.getElementById('output');
+var fieldset = document.getElementById('fieldset');
+var keygen = document.getElementById('keygen');
 var normalDiv = document.getElementById('normalDiv');
 
 shouldBe('anchor.tabIndex', '0');
@@ -33,8 +39,11 @@
 shouldBe('input.tabIndex', '0');
 shouldBe('select.tabIndex', '0');
 shouldBe('textarea.tabIndex', '0');
+shouldBe('keygen.tabIndex', '0');
 shouldBe('editableDiv.tabIndex', '0');
 shouldBe('normalDiv.tabIndex', '-1');
+shouldBe('output.tabIndex', '-1');
+shouldBe('fieldset.tabIndex', '-1');
 
 shouldBe('anchor.setAttribute("tabindex", "invalid"); anchor.tabIndex', '0');
 shouldBe('button.setAttribute("tabindex", "invalid"); button.tabIndex', '0');
@@ -41,8 +50,11 @@
 shouldBe('input.setAttribute("tabindex", "invalid"); input.tabIndex', '0');
 shouldBe('select.setAttribute("tabindex", "invalid"); select.tabIndex', '0');
 shouldBe('textarea.setAttribute("tabindex", "invalid"); textarea.tabIndex', '0');
+shouldBe('keygen.setAttribute("tabindex", "invalid"); keygen.tabIndex', '0');
 shouldBe('editableDiv.setAttribute("tabindex", "invalid"); editableDiv.tabIndex', '0');
 shouldBe('normalDiv.setAttribute("tabindex", "invalid"); normalDiv.tabIndex', '-1');
+shouldBe('output.setAttribute("tabindex", "invalid"); output.tabIndex', '-1');
+shouldBe('fieldset.setAttribute("tabindex", "invalid"); fieldset.tabIndex', '-1');
 
 shouldBe('anchor.setAttribute("tabindex", "9999999999"); anchor.tabIndex', '0');
 shouldBe('button.setAttribute("tabindex", "9999999999"); button.tabIndex', '0');
@@ -49,8 +61,11 @@
 shouldBe('input.setAttribute("tabindex", "9999999999"); input.tabIndex', '0');
 shouldBe('select.setAttribute("tabindex", "9999999999"); select.tabIndex', '0');
 shouldBe('textarea.setAttribute("tabindex", "9999999999"); textarea.tabIndex', '0');
+shouldBe('keygen.setAttribute("tabindex", "9999999999"); keygen.tabIndex', '0');
 shouldBe('editableDiv.setAttribute("tabindex", "9999999999"); editableDiv.tabIndex', '0');
 shouldBe('normalDiv.setAttribute("tabindex", "9999999999"); normalDiv.tabIndex', '-1');
+shouldBe('output.setAttribute("tabindex", "9999999999"); output.tabIndex', '-1');
+shouldBe('fieldset.setAttribute("tabindex", "9999999999"); fieldset.tabIndex', '-1');
 
 document.getElementById('container').innerHTML = '';
 </script>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlformcontrolscollection.html (248913 => 248914)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlformcontrolscollection.html	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlformcontrolscollection.html	2019-08-20 19:59:00 UTC (rev 248914)
@@ -7,8 +7,8 @@
 <script src=""
 <div id="log"></div>
 <form id="f1">
-  <input type="radio" id="r1">
-  <keygen id="kg" name="key"></keygen>
+  <input type="radio" id="r1" name="ra">
+  <keygen id="kg" name="key"></keygen> <!-- we test that it does *not* appear in form.elements -->
 </form>
 <form id="f2">
   <table>
@@ -39,7 +39,7 @@
 
 //length
 test(function () {
-  assert_equals(coll1.length, 2, "The length attribute is incorrect.");
+  assert_equals(coll1.length, 1, "The length attribute is incorrect.");
   assert_equals(coll2.length, 4, "The length attribute is incorrect.");
 }, "The length attribute must return the number of elements in the form");
 
@@ -83,17 +83,22 @@
 }, "The namedItem(name) must return null if there is no matched element");
 
 test(function () {
-  assert_equals(coll1.namedItem("kg"), document.getElementById("kg"), "Controls can be named by 'id' attribute.");
-  assert_equals(coll1.namedItem("key"), document.getElementById("kg"), "Controls can be named by 'name' attribute.");
+  assert_equals(coll1.namedItem("r1"), document.getElementById("r1"), "Controls can be named by 'id' attribute.");
+  assert_equals(coll1.namedItem("ra"), document.getElementById("r1"), "Controls can be named by 'name' attribute.");
 }, "Controls can be indexed by id or name attribute");
 
 test(function () {
+  assert_equals(coll1.namedItem("kg"), null, "Keygen does not show up when queried by id.");
+  assert_equals(coll1.namedItem("key"), null, "Keygen does not show up when queried by name.");
+}, "Keygen controls do not show up at all");
+
+test(function () {
   assert_equals(coll2.namedItem("btn").length, 2, "The length attribute should be 2.");
 }, "The namedItem(name) must return the items with id or name attribute");
 
 //various controls in fieldset and form
 var containers = ["form", "fieldset"],
-    controls = ["button", "fieldset", "input", "keygen", "object", "output", "select", "textarea"];
+    controls = ["button", "fieldset", "input", "object", "output", "select", "textarea"];
 for (var m = 0; m < containers.length; m++) {
   test(function () {
     var container = document.createElement(containers[m]);

Modified: trunk/Source/WebCore/ChangeLog (248913 => 248914)


--- trunk/Source/WebCore/ChangeLog	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/ChangeLog	2019-08-20 19:59:00 UTC (rev 248914)
@@ -1,3 +1,58 @@
+2019-08-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        The default tab index of output and fieldset should be -1
+        https://bugs.webkit.org/show_bug.cgi?id=200834
+
+        Reviewed by Alex Christensen.
+
+        This patch updates the default tab index of output and fieldset to -1 to match the behavior of
+        Chrome and Firefox as well as the latest HTML specification:
+        https://html.spec.whatwg.org/multipage/interaction.html#the-tabindex-attribute
+
+        To do that, this patch replaces HTMLFormControlElement::defaultTabIndex with defaultTabIndex
+        implementation in each one of its concrete subclass such as HTMLButtonElement to mirror
+        the language of the specification as follows:
+
+        HTMLFormControlElement
+            HTMLButtonElement -> 0
+            HTMLFieldSetElement -> -1
+            HTMLFormControlElementWithState
+                HTMLKeygenElement -> 0 - Not specified anywhere but preserving the existing behavior.
+                HTMLSelectElement -> 0
+                HTMLTextFormControlElement
+                    HTMLInputElement -> 0
+                    HTMLTextAreaElement -> 0
+            HTMLOutputElement -> -1 
+
+        Even though Element::shouldBeIgnoredInSequentialFocusNavigation() checks the value of
+        defaultTabIndex, this patch does not affect the actual sequential (tab) focus navigation:
+        Beacuse HTMLOutputElement and HTMLFieldSetElement have supportsFocus overrides to call
+        HTMLElement::supportsFocus instead of HTMLFormControlElement::supportsFocus, these two
+        elements are focusable only when tabindex is set or it's a root content editable element.
+        But all root editable elements are focusable anyway and the default tab index does not
+        matter when tabindex is set.
+
+        Test: fast/dom/tabindex-defaults.html
+
+        * html/HTMLButtonElement.cpp:
+        (WebCore::HTMLButtonElement::defaultTabIndex const): Added.
+        * html/HTMLButtonElement.h:
+        * html/HTMLFormControlElement.cpp:
+        (WebCore::HTMLFormControlElement::defaultTabIndex const): Deleted.
+        * html/HTMLFormControlElement.h:
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::defaultTabIndex const): Added.
+        * html/HTMLKeygenElement.h:
+        * html/HTMLKeygenElement.cpp:
+        (WebCore::HTMLKeygenElement::defaultTabIndex const): Added.
+        * html/HTMLKeygenElement.h:
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::defaultTabIndex const): Added.
+        * html/HTMLSelectElement.h:
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::defaultTabIndex const): Added.
+        * html/HTMLTextAreaElement.h:
+
 2019-08-20  Zalan Bujtas  <za...@apple.com>
 
         [ContentChangeObserver] isConsideredClickable should be able to process elements with no renderers

Modified: trunk/Source/WebCore/html/HTMLButtonElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLButtonElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLButtonElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -65,6 +65,11 @@
     return createRenderer<RenderButton>(*this, WTFMove(style));
 }
 
+int HTMLButtonElement::defaultTabIndex() const
+{
+    return 0;
+}
+
 const AtomString& HTMLButtonElement::formControlType() const
 {
     switch (m_type) {

Modified: trunk/Source/WebCore/html/HTMLButtonElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLButtonElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLButtonElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -51,6 +51,8 @@
 
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
 
+    int defaultTabIndex() const final;
+
     void parseAttribute(const QualifiedName&, const AtomString&) final;
     bool isPresentationAttribute(const QualifiedName&) const final;
     void defaultEventHandler(Event&) final;

Modified: trunk/Source/WebCore/html/HTMLFormControlElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLFormControlElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLFormControlElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -409,11 +409,6 @@
     return willValidate() && !isValidFormControlElement();
 }
 
-int HTMLFormControlElement::defaultTabIndex() const
-{
-    return 0;
-}
-
 bool HTMLFormControlElement::computeWillValidate() const
 {
     if (m_dataListAncestorState == Unknown) {

Modified: trunk/Source/WebCore/html/HTMLFormControlElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLFormControlElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLFormControlElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -167,8 +167,6 @@
 
     bool isFormControlElement() const final { return true; }
 
-    int defaultTabIndex() const final;
-
     bool isValidFormControlElement() const;
 
     bool computeIsDisabledByFieldsetAncestor() const;

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -435,6 +435,11 @@
     return m_inputType->hasCustomFocusLogic();
 }
 
+int HTMLInputElement::defaultTabIndex() const
+{
+    return 0;
+}
+
 bool HTMLInputElement::isKeyboardFocusable(KeyboardEvent* event) const
 {
     return m_inputType->isKeyboardFocusable(event);

Modified: trunk/Source/WebCore/html/HTMLInputElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLInputElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLInputElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -366,6 +366,7 @@
     void removedFromAncestor(RemovalType, ContainerNode&) final;
     void didMoveToNewDocument(Document& oldDocument, Document& newDocument) final;
 
+    int defaultTabIndex() const final;
     bool hasCustomFocusLogic() const final;
     bool isKeyboardFocusable(KeyboardEvent*) const final;
     bool isMouseFocusable() const final;

Modified: trunk/Source/WebCore/html/HTMLKeygenElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLKeygenElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLKeygenElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -135,6 +135,11 @@
     return keygen;
 }
 
+int HTMLKeygenElement::defaultTabIndex() const
+{
+    return 0;
+}
+
 void HTMLKeygenElement::reset()
 {
     static_cast<HTMLFormControlElement*>(shadowSelect())->reset();

Modified: trunk/Source/WebCore/html/HTMLKeygenElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLKeygenElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLKeygenElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -52,6 +52,8 @@
     bool isEnumeratable() const final { return true; }
     bool supportLabels() const final { return true; }
 
+    int defaultTabIndex() const final;
+
     void reset() final;
     bool shouldSaveAndRestoreFormControlState() const final;
 

Modified: trunk/Source/WebCore/html/HTMLSelectElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLSelectElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLSelectElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -311,6 +311,11 @@
         HTMLFormControlElementWithState::parseAttribute(name, value);
 }
 
+int HTMLSelectElement::defaultTabIndex() const
+{
+    return 0;
+}
+
 bool HTMLSelectElement::isKeyboardFocusable(KeyboardEvent* event) const
 {
     if (renderer())

Modified: trunk/Source/WebCore/html/HTMLSelectElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLSelectElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLSelectElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -111,7 +111,8 @@
 
 private:
     const AtomString& formControlType() const final;
-    
+
+    int defaultTabIndex() const final;
     bool isKeyboardFocusable(KeyboardEvent*) const final;
     bool isMouseFocusable() const final;
 

Modified: trunk/Source/WebCore/html/HTMLTextAreaElement.cpp (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLTextAreaElement.cpp	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLTextAreaElement.cpp	2019-08-20 19:59:00 UTC (rev 248914)
@@ -245,6 +245,11 @@
     return true;
 }
 
+int HTMLTextAreaElement::defaultTabIndex() const
+{
+    return 0;
+}
+
 bool HTMLTextAreaElement::isKeyboardFocusable(KeyboardEvent*) const
 {
     // If a given text area can be focused at all, then it will always be keyboard focusable.

Modified: trunk/Source/WebCore/html/HTMLTextAreaElement.h (248913 => 248914)


--- trunk/Source/WebCore/html/HTMLTextAreaElement.h	2019-08-20 19:45:43 UTC (rev 248913)
+++ trunk/Source/WebCore/html/HTMLTextAreaElement.h	2019-08-20 19:59:00 UTC (rev 248914)
@@ -113,6 +113,7 @@
     bool appendFormData(DOMFormData&, bool) final;
     void reset() final;
     bool hasCustomFocusLogic() const final;
+    int defaultTabIndex() const final;
     bool isMouseFocusable() const final;
     bool isKeyboardFocusable(KeyboardEvent*) const final;
     void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode) final;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to