Skip to content

Commit c2160b7

Browse files
mfreed7chromium-wpt-export-bot
mfreed7
authored and
chromium-wpt-export-bot
committed
Fix focus navigation loop for internal popover invokers
When a popover is opened with an invoker that is inside the popover itself (e.g. via `showPopover({source: btn})`), the focus navigation logic in FocusController previously treated that internal element as the scope owner for the popover. Because the scope owner was a descendant of the popover itself, traversing out of the popover focus scope (e.g. upon reaching the end of the popover or tabbing away) would re-resolve the popover scope from the internal owner, creating a cyclic ownership chain and causing an infinite loop or stack overflow crash. This patch updates `InvokerForOpenPopover()` and `GetOpenPopoverTarget()` to return nullptr when `FlatTreeTraversal::Contains()` finds the invoker inside the popover, preventing internal invokers from acting as scope owners. Fixed: 542274292 Change-Id: I6ff5fed25db2b4d255cb36873d5fbc480ae620da Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8265901 Reviewed-by: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1683577}
1 parent fbb56f5 commit c2160b7

1 file changed

Lines changed: 39 additions & 0 deletions

File tree

‎html/semantics/popovers/popover-focus-invoker-inside-popover.html‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8">
3+
<link rel="author" href="mailto:masonf@chromium.org">
4+
<link rel="help" href="https://crbug.com/542274292">
5+
<link rel="help" href="https://html.spec.whatwg.org/multipage/popover.html">
6+
<script src="/resources/testharness.js"></script>
7+
<script src="/resources/testharnessreport.js"></script>
8+
<script src="/resources/testdriver.js"></script>
9+
<script src="/resources/testdriver-actions.js"></script>
10+
<script src="/resources/testdriver-vendor.js"></script>
11+
<script src="resources/popover-utils.js"></script>
12+
13+
<button id="before" popovertarget="p" popovertargetaction="show">Click me first</button>
14+
<div id="p" popover>
15+
<button id="b" popovertarget="p" tabindex="-1">Close</button>
16+
<button id="inside">Click me second, THEN press tab</button>
17+
</div>
18+
<button id="after">After</button>
19+
20+
<script>
21+
promise_test(async () => {
22+
p.showPopover({source: b});
23+
assert_true(p.matches(':popover-open'), 'popover should be open');
24+
before.focus();
25+
assert_equals(document.activeElement, before, 'focus starts on before button');
26+
27+
await clickOn(before);
28+
assert_true(p.matches(':popover-open'), 'popover should stay open');
29+
assert_equals(document.activeElement, before, 'focus should remain on before button');
30+
31+
await clickOn(inside);
32+
assert_true(p.matches(':popover-open'), 'popover should stay open');
33+
assert_equals(document.activeElement, inside, 'focus should move to inside button');
34+
35+
await sendTab();
36+
assert_true(p.matches(':popover-open'), 'popover should stay open');
37+
assert_equals(document.activeElement, after, 'focus should move to after button');
38+
}, "Popover with an internal tabindex=-1 invoker does not cause infinite recursion");
39+
</script>

0 commit comments

Comments
 (0)