Fix several Element#tabIndex() cases

packages/alfa-dom/src/node/element.ts

@@ -217,7 +217,7 @@ export class Element extends Node implements Slot, Slotable {
       }
     }
 
-    if (isSuggestedFocusableElement(this)) {
+    if (Element.isSuggestedFocusable(this)) {
       return Some.of(0);
     }
 
@@ -376,72 +376,14 @@ export namespace Element {
     hasName,
     hasNamespace,
     hasTabIndex,
+    isBrowsingContextContainer,
     isDisabled,
+    isDraggable,
+    isEditingHost,
+    isSuggestedFocusable,
   } = predicate;
 }
 
 function indent(input: string): string {
   return input.replace(/^/gm, "  ");
 }
-
-function isSuggestedFocusableElement(element: Element): boolean {
-  switch (element.name) {
-    case "a":
-    case "link":
-      return element.attribute("href").isSome();
-
-    case "input":
-      return element
-        .attribute("type")
-        .flatMap((attribute) => attribute.enumerate("hidden"))
-        .isNone();
-
-    case "audio":
-    case "video":
-      return element.attribute("controls").isSome();
-
-    case "button":
-    case "select":
-    case "textarea":
-      return true;
-
-    case "summary":
-      return element
-        .parent()
-        .filter(Element.isElement)
-        .some((parent) => {
-          if (parent.name === "details") {
-            for (const child of parent.children()) {
-              if (Element.isElement(child) && child.name === "summary") {
-                return child === element;
-              }
-            }
-          }
-
-          return false;
-        });
-
-    case "object":
-      return element.content.isSome();
-  }
-
-  return (
-    element.attribute("draggable").isSome() ||
-    isEditingHost(element) ||
-    isBrowsingContextContainer(element)
-  );
-}
-
-/**
- * @see https://html.spec.whatwg.org/#editing-host
- */
-function isEditingHost(element: Element): boolean {
-  return element.attribute("contenteditable").isSome();
-}
-
-/**
- * @see https://html.spec.whatwg.org/#browsing-context-container
- */
-function isBrowsingContextContainer(element: Element): boolean {
-  return element.name === "iframe";
-}

packages/alfa-dom/src/node/element/predicate.ts

@@ -3,4 +3,8 @@ export * from "./predicate/has-input-type";
 export * from "./predicate/has-name";
 export * from "./predicate/has-namespace";
 export * from "./predicate/has-tab-index";
+export * from "./predicate/is-browsing-context-container";
 export * from "./predicate/is-disabled";
+export * from "./predicate/is-draggable";
+export * from "./predicate/is-editing-host";
+export * from "./predicate/is-suggested-focusable";

packages/alfa-dom/src/node/element/predicate/is-browsing-context-container.ts

@@ -0,0 +1,21 @@
+import { Element } from "../../element";
+
+/**
+ * @see https://html.spec.whatwg.org/#browsing-context-container
+ */
+export function isBrowsingContextContainer(element: Element): boolean {
+  switch (element.name) {
+    // <iframe> elements are _always_ browsing context containers as they will
+    // _always_ have a content document, although it might be empty.
+    case "iframe":
+      return true;
+
+    // <object> elements are only browsing context containers if they have a
+    // content document.
+    case "object":
+      return element.content.isSome();
+
+    default:
+      return false;
+  }
+}

packages/alfa-dom/src/node/element/predicate/is-disabled.ts

@@ -9,7 +9,7 @@ const { equals } = Predicate;
 /**
  * @see https://html.spec.whatwg.org/#concept-fe-disabled
  */
-export const isDisabled: Predicate<Element> = (element) => {
+export function isDisabled(element: Element): boolean {
   switch (element.name) {
     // https://html.spec.whatwg.org/#attr-fe-disabled
     case "button":
@@ -53,4 +53,4 @@ export const isDisabled: Predicate<Element> = (element) => {
   }
 
   return false;
-};
+}

packages/alfa-dom/src/node/element/predicate/is-draggable.ts

@@ -0,0 +1,33 @@
+import { Element } from "../../element";
+
+/**
+ * @see https://html.spec.whatwg.org/#dom-draggable
+ */
+export function isDraggable(element: Element): boolean {
+  return element
+    .attribute("draggable")
+    .map((attribute) =>
+      attribute.enumerate("true", "false", "auto").getOr("auto")
+    )
+    .some((draggable) => {
+      switch (draggable) {
+        case "true":
+          return true;
+
+        case "false":
+          return false;
+
+        case "auto":
+          switch (element.name) {
+            case "img":
+              return true;
+
+            case "a":
+              return element.attribute("href").isSome();
+
+            default:
+              return false;
+          }
+      }
+    });
+}

packages/alfa-dom/src/node/element/predicate/is-editing-host.ts

@@ -0,0 +1,20 @@
+import { Element } from "../../element";
+
+/**
+ * @see https://html.spec.whatwg.org/#editing-host
+ */
+export function isEditingHost(element: Element): boolean {
+  return element
+    .attribute("contenteditable")
+    .flatMap((attribute) => attribute.enumerate("", "true", "false"))
+    .some((contenteditable) => {
+      switch (contenteditable) {
+        case "":
+        case "true":
+          return true;
+
+        case "false":
+          return false;
+      }
+    });
+}

packages/alfa-dom/src/node/element/predicate/is-suggested-focusable.ts

@@ -0,0 +1,52 @@
+import { Element } from "../../element";
+
+/**
+ * @see https://html.spec.whatwg.org/#tabindex-value
+ *
+ * @remarks
+ * Draggable elements for which the user agent supports drag operations without
+ * a pointer device are also suggested focusable. However, we're not aware of
+ * any such cases and therefore don't suggest making draggable elements
+ * focusable.
+ */
+export function isSuggestedFocusable(element: Element): boolean {
+  switch (element.name) {
+    case "a":
+    case "link":
+      return element.attribute("href").isSome();
+
+    case "input":
+      return element
+        .attribute("type")
+        .flatMap((attribute) => attribute.enumerate("hidden"))
+        .isNone();
+
+    case "audio":
+    case "video":
+      return element.attribute("controls").isSome();
+
+    case "button":
+    case "select":
+    case "textarea":
+      return true;
+
+    case "summary":
+      return element
+        .parent()
+        .filter(Element.isElement)
+        .some(
+          (parent) =>
+            parent.name === "details" &&
+            parent
+              .children()
+              .filter(Element.isElement)
+              .find(Element.hasName("summary"))
+              .includes(element)
+        );
+  }
+
+  return (
+    Element.isEditingHost(element) ||
+    Element.isBrowsingContextContainer(element)
+  );
+}

packages/alfa-dom/test/node/element.spec.tsx

@@ -0,0 +1,45 @@
+import { test } from "@siteimprove/alfa-test";
+
+import { jsx } from "../../jsx";
+
+test("#tabIndex() returns the tab index explicitly assigned to an element", (t) => {
+  t.equal((<div tabindex="42" />).tabIndex().get(), 42);
+});
+
+test("#tabIndex() returns 0 for an <a> element with an href attribute", (t) => {
+  t.equal((<a href="#" />).tabIndex().get(), 0);
+});
+
+test("#tabIndex() returns None for an <a> element without an href attribute", (t) => {
+  t.equal((<a />).tabIndex().isNone(), true);
+});
+
+test("#tabIndex() returns 0 for a <button> element", (t) => {
+  t.equal((<button />).tabIndex().get(), 0);
+});
+
+test(`#tabIndex() returns 0 for a <summary> element that is the first <summary>
+      child element of a <details> element`, (t) => {
+  const summary = <summary />;
+
+  <details>{summary}</details>;
+
+  t.equal(summary.tabIndex().get(), 0);
+});
+
+test(`#tabIndex() returns None for a <summary> element that is not the child of
+      a <details> elements`, (t) => {
+  t.equal((<summary />).tabIndex().isNone(), true);
+});
+
+test(`#tabIndex() returns None for a <summary> element that is not the first
+      <summary> child element of a <details> elements`, (t) => {
+  const summary = <summary />;
+
+  <details>
+    <summary />
+    {summary}
+  </details>;
+
+  t.equal(summary.tabIndex().isNone(), true);
+});

packages/alfa-dom/tsconfig.json

@@ -19,7 +19,11 @@
     "src/node/element/predicate/has-name.ts",
     "src/node/element/predicate/has-namespace.ts",
     "src/node/element/predicate/has-tab-index.ts",
+    "src/node/element/predicate/is-browsing-context-container.ts",
     "src/node/element/predicate/is-disabled.ts",
+    "src/node/element/predicate/is-draggable.ts",
+    "src/node/element/predicate/is-editing-host.ts",
+    "src/node/element/predicate/is-suggested-focusable.ts",
     "src/node/fragment.ts",
     "src/node/shadow.ts",
     "src/node/slot.ts",
@@ -42,7 +46,8 @@
     "src/style/rule/supports.ts",
     "src/style/sheet.ts",
     "test/h.spec.ts",
-    "test/node/attribute.spec.ts"
+    "test/node/attribute.spec.ts",
+    "test/node/element.spec.tsx"
   ],
   "references": [
     {

packages/alfa-rules/test/sia-r2/rule.spec.tsx

@@ -32,20 +32,28 @@ test("evaluate() fails an <img> element without an accessible name", async (t) =
   ]);
 });
 
-test("evaluate() is inapplicable to a document without images", async (t) => {
-  const document = Document.empty();
+test("evaluate() is inapplicable to an <img> element with a presentational role", async (t) => {
+  const document = Document.of([<img role="none"></img>]);
 
   t.deepEqual(await evaluate(R2, { document }), [inapplicable(R2)]);
 });
 
-test("evaluate() is inapplicable to an image that is marked as decorative", async (t) => {
-  const document = Document.of([<img role="none"></img>]);
+test("evaluate() is inapplicable to an <img> element that is hidden", async (t) => {
+  const document = Document.of([<img hidden alt="Hello world"></img>]);
 
   t.deepEqual(await evaluate(R2, { document }), [inapplicable(R2)]);
 });
 
-test("evaluate() is inapplicable to an image that is hidden", async (t) => {
-  const document = Document.of([<img hidden alt="Hello world"></img>]);
+test("evaluate() is inapplicable to a document without images", async (t) => {
+  const document = Document.empty();
+
+  t.deepEqual(await evaluate(R2, { document }), [inapplicable(R2)]);
+});
+
+// https://github.com/siteimprove/alfa/issues/444
+test(`evaluate() is inapplicable to a non-draggable <img> element with a
+      presentational role`, async (t) => {
+  const document = Document.of([<img role="none" draggable="false"></img>]);
 
   t.deepEqual(await evaluate(R2, { document }), [inapplicable(R2)]);
 });