From 3b282b644f238dcf8811992e48d2a1068d154b83 Mon Sep 17 00:00:00 2001
From: Jean-Yves Moyen <jym@siteimprove.com>
Date: Tue, 1 Jun 2021 09:20:18 +0200
Subject: [PATCH] SIA R65: accept different border as focus indicator (#819)

* Remove duplicate test on border-style
* Accept difference in border-* as focus indicator
* Add some tests
---
 .../src/common/predicate/has-border.ts        |  3 -
 packages/alfa-rules/src/sia-r65/rule.ts       | 59 ++++++++++++++++++-
 .../alfa-rules/test/sia-r65/rule.spec.tsx     | 55 +++++++++++++++++
 3 files changed, 112 insertions(+), 5 deletions(-)

diff --git a/packages/alfa-rules/src/common/predicate/has-border.ts b/packages/alfa-rules/src/common/predicate/has-border.ts
index 1bdbdac476..fb5aca50cc 100644
--- a/packages/alfa-rules/src/common/predicate/has-border.ts
+++ b/packages/alfa-rules/src/common/predicate/has-border.ts
@@ -17,9 +17,6 @@ export function hasBorder(
     return sides.some(
       (side) =>
         style.computed(`border-${side}-width` as const).none(Length.isZero) &&
-        style
-          .computed(`border-${side}-style` as const)
-          .none((style) => style.value === "none") &&
         style
           .computed(`border-${side}-color` as const)
           .none((color) => color.type === "color" && Color.isTransparent(color))
diff --git a/packages/alfa-rules/src/sia-r65/rule.ts b/packages/alfa-rules/src/sia-r65/rule.ts
index 2298b858f8..aaf0feccfb 100644
--- a/packages/alfa-rules/src/sia-r65/rule.ts
+++ b/packages/alfa-rules/src/sia-r65/rule.ts
@@ -12,6 +12,7 @@ import { Page } from "@siteimprove/alfa-web";
 import { expectation } from "../common/expectation";
 
 import {
+  hasBorder,
   hasBoxShadow,
   hasOutline,
   hasTextDecoration,
@@ -22,7 +23,7 @@ import { Question } from "../common/question";
 
 const { isElement } = Element;
 const { isKeyword } = Keyword;
-const { or, xor } = Predicate;
+const { or, test, xor } = Predicate;
 
 export default Rule.Atomic.of<Page, Element, Question>({
   uri: "https://alfa.siteimprove.com/rules/sia-r65",
@@ -99,7 +100,9 @@ function hasFocusIndicator(device: Device): Predicate<Element> {
           xor(hasBoxShadow(device), hasBoxShadow(device, withFocus)),
           // These properties are essentially always set, so any difference in the color is good enough.
           hasDifferentColors(device, withFocus),
-          hasDifferentBackgroundColors(device, withFocus)
+          hasDifferentBackgroundColors(device, withFocus),
+          // Any difference in border is accepted
+          hasDifferentBorder(device, withFocus)
         )
       );
   };
@@ -148,3 +151,55 @@ function hasDifferentBackgroundColors(
     return !color1.equals(color2);
   };
 }
+
+function hasDifferentBorder(
+  device: Device,
+  context1: Context = Context.empty(),
+  context2: Context = Context.empty()
+): Predicate<Element> {
+  return function hasDifferentBorder(element: Element): boolean {
+    const style1 = Style.from(element, device, context1);
+    const style2 = Style.from(element, device, context2);
+
+    // If 0 or 1 has border, the answer is easy.
+    const hasBorder1 = test(hasBorder(device, context1), element);
+    const hasBorder2 = test(hasBorder(device, context2), element);
+
+    if (hasBorder1 !== hasBorder2) {
+      // only one has border
+      return true;
+    }
+
+    if (!hasBorder1 && !hasBorder2) {
+      // none has border
+      return false;
+    }
+
+    // They both have border, we need to dig the values
+
+    // We consider any difference in any of the border-* longhand as enough
+    for (const side of ["top", "right", "bottom", "left"] as const) {
+      for (const effect of ["color", "style", "width"] as const) {
+        const longhand = `border-${side}-${effect}` as const;
+
+        const border1 = style1.computed(longhand);
+        const border2 = style2.computed(longhand);
+
+        // We avoid keyword resolution for color,
+        // but we need it for style. The none=hidden conflict has been solved
+        // by hasBorder so any difference in style is enough.
+        if (
+          !(
+            (effect === "color" &&
+              (isKeyword(border1) || isKeyword(border2))) ||
+            border1.equals(border2)
+          )
+        ) {
+          return true;
+        }
+      }
+    }
+
+    return false;
+  };
+}
diff --git a/packages/alfa-rules/test/sia-r65/rule.spec.tsx b/packages/alfa-rules/test/sia-r65/rule.spec.tsx
index 578f13dc05..4b288a5598 100644
--- a/packages/alfa-rules/test/sia-r65/rule.spec.tsx
+++ b/packages/alfa-rules/test/sia-r65/rule.spec.tsx
@@ -365,3 +365,58 @@ test(`evaluate() passes an <a> element that removes the default focus outline
     }),
   ]);
 });
+
+test(`evaluate() passes an <a> element that removes the default focus outline
+      and applies a border on focus`, async (t) => {
+  const target = <a href="#">Link</a>;
+
+  const document = Document.of(
+    [target, <button />],
+    [
+      h.sheet([
+        h.rule.style("a:focus", {
+          outline: "none",
+          border: "solid 1px",
+        }),
+      ]),
+    ]
+  );
+
+  t.deepEqual(await evaluate(R65, { document }), [
+    passed(R65, target, {
+      1: Outcomes.HasFocusIndicator,
+    }),
+    passed(R65, <button />, {
+      1: Outcomes.HasFocusIndicator,
+    }),
+  ]);
+});
+
+test(`evaluate() passes an <a> element that removes the default focus outline
+      and changes border color on focus`, async (t) => {
+  const target = <a href="#">Link</a>;
+
+  const document = Document.of(
+    [target, <button />],
+    [
+      h.sheet([
+        h.rule.style("a", {
+          border: "solid 1px black",
+        }),
+        h.rule.style("a:focus", {
+          outline: "none",
+          border: "solid 1px red",
+        }),
+      ]),
+    ]
+  );
+
+  t.deepEqual(await evaluate(R65, { document }), [
+    passed(R65, target, {
+      1: Outcomes.HasFocusIndicator,
+    }),
+    passed(R65, <button />, {
+      1: Outcomes.HasFocusIndicator,
+    }),
+  ]);
+});