Bug 1589027 - Fix object.tabIndex should return 0 by default; r=smaug

The spec change HTMLObjectElement tabIndex default to 0, but `test_object_plugin_nav.html` will fail due to HTMLObjectElement’s sequence focus and .focus() rely on default tabIndex. In this patch I separate the logic to not rely on default tabIndex for HTMLObjectElement's plugin type and align the spec.

Differential Revision: https://phabricator.services.mozilla.com/D50801

--HG--
extra : moz-landing-system : lando

dom/html/HTMLObjectElement.cpp

@@ -276,17 +276,6 @@ nsresult HTMLObjectElement::AfterMaybeChangeAttr(int32_t aNamespaceID,
   return NS_OK;
 }
 
-bool HTMLObjectElement::IsFocusableForTabIndex() {
-  Document* doc = GetComposedDoc();
-  if (!doc || doc->HasFlag(NODE_IS_EDITABLE)) {
-    return false;
-  }
-
-  return IsEditableRoot() ||
-         ((Type() == eType_Document || Type() == eType_FakePlugin) &&
-          nsContentUtils::IsSubDocumentTabbable(this));
-}
-
 bool HTMLObjectElement::IsHTMLFocusable(bool aWithMouse, bool* aIsFocusable,
                                         int32_t* aTabIndex) {
   // TODO: this should probably be managed directly by IsHTMLFocusable.
@@ -294,38 +283,45 @@ bool HTMLObjectElement::IsHTMLFocusable(bool aWithMouse, bool* aIsFocusable,
   Document* doc = GetComposedDoc();
   if (!doc || doc->HasFlag(NODE_IS_EDITABLE)) {
     if (aTabIndex) {
-      *aTabIndex = TabIndex();
+      *aTabIndex = -1;
     }
 
     *aIsFocusable = false;
+    return false;
+  }
 
+  const nsAttrValue* attrVal = mAttrs.GetAttr(nsGkAtoms::tabindex);
+  bool isFocusable = attrVal && attrVal->Type() == nsAttrValue::eInteger;
+
+  // Has plugin content: let the plugin decide what to do in terms of
+  // internal focus from mouse clicks
+  if (Type() == eType_Plugin) {
+    if (aTabIndex) {
+      *aTabIndex = isFocusable ? attrVal->GetIntegerValue() : -1;
+    }
+
+    *aIsFocusable = true;
     return false;
   }
 
   // This method doesn't call nsGenericHTMLFormElement intentionally.
   // TODO: It should probably be changed when bug 597242 will be fixed.
-  if (Type() == eType_Plugin || IsEditableRoot() ||
+  if (IsEditableRoot() ||
       ((Type() == eType_Document || Type() == eType_FakePlugin) &&
        nsContentUtils::IsSubDocumentTabbable(this))) {
-    // Has plugin content: let the plugin decide what to do in terms of
-    // internal focus from mouse clicks
     if (aTabIndex) {
-      *aTabIndex = TabIndex();
+      *aTabIndex = isFocusable ? attrVal->GetIntegerValue() : 0;
     }
 
     *aIsFocusable = true;
-
     return false;
   }
 
   // TODO: this should probably be managed directly by IsHTMLFocusable.
   // See bug 597242.
-  const nsAttrValue* attrVal = mAttrs.GetAttr(nsGkAtoms::tabindex);
-
-  *aIsFocusable = attrVal && attrVal->Type() == nsAttrValue::eInteger;
-
-  if (aTabIndex && *aIsFocusable) {
+  if (aTabIndex && isFocusable) {
     *aTabIndex = attrVal->GetIntegerValue();
+    *aIsFocusable = true;
   }
 
   return false;
@@ -372,9 +368,7 @@ HTMLObjectElement::SubmitNamesValues(HTMLFormSubmission* aFormSubmission) {
   return aFormSubmission->AddNameValuePair(name, value);
 }
 
-int32_t HTMLObjectElement::TabIndexDefault() {
-  return IsFocusableForTabIndex() ? 0 : -1;
-}
+int32_t HTMLObjectElement::TabIndexDefault() { return 0; }
 
 Nullable<WindowProxyHolder> HTMLObjectElement::GetContentWindow(
     nsIPrincipal& aSubjectPrincipal) {

dom/html/HTMLObjectElement.h

@@ -198,12 +198,6 @@ class HTMLObjectElement final : public nsGenericHTMLFormElement,
                                           bool aNotify) override;
 
  private:
-  /**
-   * Returns if the element is currently focusable regardless of it's tabindex
-   * value. This is used to know the default tabindex value.
-   */
-  bool IsFocusableForTabIndex();
-
   nsContentPolicyType GetContentPolicyType() const override {
     return nsIContentPolicy::TYPE_INTERNAL_OBJECT;
   }

dom/html/test/test_bug586763.html

@@ -31,7 +31,7 @@ var tests = [
   ["area", "tabIndex", 0],
   ["button", "tabIndex", 0],
   ["input", "tabIndex", 0],
-  ["object", "tabIndex", -1],
+  ["object", "tabIndex", 0],
   ["select", "tabIndex", 0],
   ["textarea", "tabIndex", 0],
 ];

dom/html/test/test_bug596350.html

@@ -26,13 +26,13 @@ addLoadEvent(runTests);
 
 var testData = [
 // Object 0
-  [ 0, null, -1 ],
+  [ 0, null, 0 ],
   [ 0, "1", 1 ],
   [ 0, "-1", -1 ],
   [ 0, "0", 0 ],
-  [ 0, "foo", -1 ],
+  [ 0, "foo", 0 ],
 // Object 1
-  [ 1, null, -1 ],
+  [ 1, null, 0 ],
   [ 1, "1", 1 ],
 // Object 2
   [ 2, null, 0 ],

dom/html/test/test_object_plugin_nav.html

@@ -62,8 +62,8 @@ SimpleTest.waitForExplicitFinish();
 function doTest() {
   is(document.activeElement, document.body);
 
-  // Preliminary check: tabindex should be -1 on the object.
-  is(document.getElementsByTagName('object')[0].tabIndex, -1,
+  // Preliminary check: tabindex should be 0 on the object.
+  is(document.getElementsByTagName('object')[0].tabIndex, 0,
      "the plugin shouldn't get focus while navigating in the document");
 
   document.addEventListener("focus", function() {

testing/web-platform/meta/html/interaction/focus/sequential-focus-navigation-and-the-tabindex-attribute/tabindex-getter.html.ini

@@ -1,4 +0,0 @@
-[tabindex-getter.html]
-  [object.tabIndex should return 0 by default]
-    expected: FAIL
-