From e9a622b668a830fca425a164071ffd9691b19d0a Mon Sep 17 00:00:00 2001
From: Mason Freed <masonf@chromium.org>
Date: Wed, 9 Nov 2022 14:41:20 -0800
Subject: [PATCH] Implement correct focus behavior for popovers

This implements the focus behavior described in [1], which:
 1. Moves focus from an invoking element to its invoked popover,
    regardless of where in the DOM that popover lives.
 2. Moves focus back to the next focusable element after the
    invoking element once focus leaves the invoked popover.
 3. Skips over an open invoked popover otherwise.

The logic follows very closely the case of slotted light DOM content,
for which focus moves from the shadow root content to the slotted
light DOM content and back.

[1] https://github.com/w3c/html-aam/wiki/HTML-Popup-Attribute-A11y-Proposal-%28manual-and-auto%29

Bug: 1307772
Change-Id: Ic12441fc3b8d2f1c405bf912234dd24e4b05dc69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4006714
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069375}
---
 .../popovers/popover-focus.tentative.html     | 80 +++++++++++++++++++
 .../popover-light-dismiss.tentative.html      | 35 --------
 .../popovers/resources/popover-utils.js       | 25 +++++-
 3 files changed, 102 insertions(+), 38 deletions(-)

diff --git a/html/semantics/popovers/popover-focus.tentative.html b/html/semantics/popovers/popover-focus.tentative.html
index 10c91bcebba7b4..0d35281164c3b9 100644
--- a/html/semantics/popovers/popover-focus.tentative.html
+++ b/html/semantics/popovers/popover-focus.tentative.html
@@ -266,3 +266,83 @@
 
   document.querySelectorAll('body > [popover]').forEach(popover => activateAndVerify(popover));
 </script>
+
+<div id=fixup>
+  <button id=button1>Button1</button>
+  <div popover id=popover1 style="top:100px">
+    <button id=inside_popover1>Inside1</button>
+    <button id=invoker2 popovertoggletarget=popover2>Nested Invoker 2</button>
+    <button id=inside_popover2>Inside2</button>
+  </div>
+  <button id=button2>Button2</button>
+  <button popovertoggletarget=popover1 id=invoker1>Invoker1</button>
+  <button id=button3>Button3</button>
+  <div popover id=popover2 style="top:200px">
+    <button id=inside_popover3>Inside3</button>
+    <button id=invoker3 popovertoggletarget=popover3>Nested Invoker 3</button>
+  </div>
+  <div popover id=popover3 style="top:300px">
+    Non-focusable popover
+  </div>
+  <button id=button4>Button4</button>
+</div>
+<style>
+  #fixup [popover] {
+    bottom:auto;
+  }
+</style>
+<script>
+async function verifyFocusOrder(order) {
+  order[0].focus();
+  for(let i=0;i<order.length;++i) {
+    const control = order[i];
+    assert_equals(document.activeElement,control,`Step ${i+1}`);
+    await sendTab();
+  }
+  // Shift-tab not supported, crbug.com/893480.
+  // for(let i=order.length-1;i>=0;--i) {
+  //   const control = order[i];
+  //   await sendShiftTab();
+  //   assert_equals(document.activeElement,control,`Step ${i+1} (backwards)`);
+  // }
+}
+promise_test(async t => {
+  button1.focus();
+  assert_equals(document.activeElement,button1);
+  await sendTab();
+  assert_equals(document.activeElement,button2,'Hidden popover should be skipped');
+  // Shift-tab not supported, crbug.com/893480.
+  // await sendShiftTab();
+  // assert_equals(document.activeElement,button1,'Hidden popover should be skipped backwards');
+  //await sendTab();
+  await sendTab();
+  assert_equals(document.activeElement,invoker1);
+  await sendEnter(); // Activate the invoker
+  assert_true(popover1.matches(':open'), 'popover1 should be invoked by invoker1');
+  assert_equals(document.activeElement,invoker1,'Focus should not move when popover is shown');
+  await sendTab();
+  assert_equals(document.activeElement,inside_popover1,'Focus should move from invoker into the open popover');
+  await sendTab();
+  assert_equals(document.activeElement,invoker2,'Focus should move within popover');
+  await verifyFocusOrder([button1, button2, invoker1, inside_popover1, invoker2, inside_popover2, button3, button4]);
+  invoker2.focus();
+  await sendEnter(); // Activate the nested invoker
+  assert_true(popover2.matches(':open'), 'popover2 should be invoked by nested invoker');
+  assert_equals(document.activeElement,invoker2,'Focus should stay on the invoker');
+  await sendTab();
+  assert_equals(document.activeElement,inside_popover3,'Focus should move into nested popover');
+  await sendTab();
+  assert_equals(document.activeElement,invoker3);
+  await sendEnter(); // Activate the (empty) nested invoker
+  assert_true(popover3.matches(':open'), 'popover3 should be invoked by nested invoker');
+  assert_equals(document.activeElement,invoker3,'Focus should stay on the invoker');
+  await sendTab();
+  assert_equals(document.activeElement,inside_popover2,'Focus should skip popover without focusable content, going back to higher scope');
+  await sendTab();
+  assert_equals(document.activeElement,button3,'Focus should exit popovers');
+  await sendTab();
+  assert_equals(document.activeElement,button4,'Focus should skip popovers');
+  button1.focus();
+  await verifyFocusOrder([button1, button2, invoker1, inside_popover1, invoker2, inside_popover3, invoker3, inside_popover2, button3, button4]);
+}, "Popover focus navigation");
+</script>
diff --git a/html/semantics/popovers/popover-light-dismiss.tentative.html b/html/semantics/popovers/popover-light-dismiss.tentative.html
index 2581ca6fdeea0f..2a101483cb8c8e 100644
--- a/html/semantics/popovers/popover-light-dismiss.tentative.html
+++ b/html/semantics/popovers/popover-light-dismiss.tentative.html
@@ -389,41 +389,6 @@
   },'Moving focus back to the anchor element should not dismiss the popover');
 </script>
 
-<div popover id=p9>
-  <button>Button</button>
-  <span id=inside9after>Inside popover 9 after button</span>
-</div>
-<button id=b9after popovertoggletarget='p9'>Popover 9</button>
-<script>
-  promise_test(async () => {
-    const popover9 = document.querySelector('#p9');
-    const inside9After = document.querySelector('#inside9after');
-    const popover9Invoker = document.querySelector('#b9after');
-    assert_false(popover9.matches(':open'));
-    popover9Invoker.click(); // Trigger via the button
-    await clickOn(inside9After);
-    assert_true(popover9.matches(':open'));
-    await sendTab();
-    assert_equals(document.activeElement,popover9Invoker,'Focus should move to the invoking element');
-    assert_true(popover9.matches(':open'),'popover should stay open');
-    popover9.hidePopover();
-  },'Moving focus back to the active trigger element should not dismiss the popover');
-
-  promise_test(async () => {
-    const popover9 = document.querySelector('#p9');
-    const inside9After = document.querySelector('#inside9after');
-    const popover9Invoker = document.querySelector('#b9after');
-    assert_false(popover9.matches(':open'));
-    popover9.showPopover(); // Trigger directly
-    await clickOn(inside9After);
-    assert_true(popover9.matches(':open'));
-    await sendTab();
-    assert_equals(document.activeElement,popover9Invoker,'Focus should move to the invoking element');
-    assert_true(popover9.matches(':open'),'popover should stay open - even though the trigger wasn\'t used, it points to this popover');
-  },'Moving focus back to an inactive trigger element should also *not* dismiss the popover');
-</script>
-
-
 <!-- Convoluted ancestor relationship -->
 <div popover id=convoluted_p1>Popover 1
   <div id=convoluted_anchor>Anchor
diff --git a/html/semantics/popovers/resources/popover-utils.js b/html/semantics/popovers/resources/popover-utils.js
index 8f66dc0844975d..efc318b3c9fd34 100644
--- a/html/semantics/popovers/resources/popover-utils.js
+++ b/html/semantics/popovers/resources/popover-utils.js
@@ -12,14 +12,33 @@ async function clickOn(element) {
 }
 async function sendTab() {
   await waitForRender();
-  await new test_driver.send_keys(document.body,'\uE004'); // Tab
-  await waitForRender();
-}
+  const kTab = '\uE004';
+  await new test_driver.send_keys(document.body,kTab);
+  await waitForRender();
+}
+// Waiting for crbug.com/893480:
+// async function sendShiftTab() {
+//   await waitForRender();
+//   const kShift = '\uE008';
+//   const kTab = '\uE004';
+//   await new test_driver.Actions()
+//     .keyDown(kShift)
+//     .keyDown(kTab)
+//     .keyUp(kTab)
+//     .keyUp(kShift)
+//     .send();
+//   await waitForRender();
+// }
 async function sendEscape() {
   await waitForRender();
   await new test_driver.send_keys(document.body,'\uE00C'); // Escape
   await waitForRender();
 }
+async function sendEnter() {
+  await waitForRender();
+  await new test_driver.send_keys(document.body,'\uE007'); // Enter
+  await waitForRender();
+}
 function isElementVisible(el) {
   return !!(el.offsetWidth || el.offsetHeight || el.getClientRects().length);
 }