Skip to content

Commit

Permalink
Add a test for returning popover focus after hide
Browse files Browse the repository at this point in the history
This was raised as an issue - it is possible that a triggering element
within a popover (with a popoverhidetarget trigger) might not return
focus to the previously focused element. The new test verifies that
focus is properly returned. This CL also refactores another focus test
out into the new file, to avoid conflict with the animations in the
popover-focus text.

Fixed: 1371629
Bug: 1307772
Change-Id: I425b1dfe2c14f498c6d10e522d8f428fd03379e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4010620
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1069528}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Nov 10, 2022
1 parent af4dd28 commit 1d94b36
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 80 deletions.
113 changes: 113 additions & 0 deletions html/semantics/popovers/popover-focus-2.tentative.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<!DOCTYPE html>
<meta charset="utf-8" />
<title>Popover focus behaviors</title>
<link rel="author" href="mailto:[email protected]">
<link rel=help href="https://open-ui.org/components/popover.research.explainer">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/popover-utils.js"></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>

<div id=deleted>
<button popovershowtarget=deleted1>Show popover</button>
<div popover id=deleted1>
<button popoverhidetarget=deleted1 autofocus>Hide popover</button>
</div>
</div>
<script>
promise_test(async t => {
const invoker = document.querySelector('#deleted>button');
const popover = document.querySelector('#deleted>[popover]');
const hideButton = popover.querySelector('[popoverhidetarget]');
invoker.focus(); // Make sure button is focused.
assert_equals(document.activeElement,invoker);
await sendEnter(); // Activate the invoker
assert_true(popover.matches(':open'), 'popover should be invoked by invoker');
assert_equals(document.activeElement,hideButton,'Hide button should be focused due to autofocus attribute');
await sendEnter(); // Activate the hide invoker
assert_false(popover.matches(':open'), 'popover should be hidden by invoker');
assert_equals(document.activeElement,invoker,'Focus should be returned to the invoker');
}, "Popover focus returns when popover is hidden by invoker");
</script>
80 changes: 0 additions & 80 deletions html/semantics/popovers/popover-focus.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -266,83 +266,3 @@

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>

0 comments on commit 1d94b36

Please sign in to comment.