Skip to content

Simplify popover=hint behaviours & related spec fixes#12345

Open
jakearchibald wants to merge 12 commits intomainfrom
jakearchibald/popover-hint-simpler-behaviours
Open

Simplify popover=hint behaviours & related spec fixes#12345
jakearchibald wants to merge 12 commits intomainfrom
jakearchibald/popover-hint-simpler-behaviours

Conversation

@jakearchibald
Copy link
Copy Markdown
Collaborator

@jakearchibald jakearchibald commented Apr 8, 2026

  • At least two implementers are interested (and none opposed):
    • Mozilla
    • Google
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • Corresponding HTML AAM & ARIA in HTML issues & PRs:
  • MDN issue is filed: …
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

Fixes #12304


/infrastructure.html ( diff )
/interactive-elements.html ( diff )
/popover.html ( diff )

Comment thread source
<p>To <dfn export data-x="hide-all-popovers-until">hide all popovers until</dfn>, given an <span
data-x="HTML elements">HTML element</span> or <code>Document</code> <var>endpoint</var>, a boolean
<var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>
<p>To <dfn>hide popover stack until</dfn>, given an <span data-x="html elements">HTML
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed a rewrite. The previous version looped the wrong way, and assumed popoverList was 'live'.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and sorry.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nahhhhh if anything went wrong here it was in review.

@jakearchibald
Copy link
Copy Markdown
Collaborator Author

jakearchibald commented Apr 8, 2026

I'll sort out the wrapping after reviews & changes.

@jakearchibald jakearchibald marked this pull request as ready for review April 8, 2026 14:01
@lukewarlow
Copy link
Copy Markdown
Member

Btw https://github.com/domfarolino/specfmt is quite useful for the wrapping.

@jakearchibald
Copy link
Copy Markdown
Collaborator Author

@lukewarlow haha your deleted comment did reveal a bug! It wasn't the return false that was wrong, but I was missing a return true.

@jakearchibald
Copy link
Copy Markdown
Collaborator Author

jakearchibald commented Apr 8, 2026

Edit: I've broken this out into #12355

av1.mp4

@mfreed7 sorry for another video, but I don't think I'd be able to describe it well. What do you see as the intended behaviour here?

https://random-stuff.jakearchibald.com/bug-repros/popover-show-on-hide-warning/ - here's the demo page.

Edit: I'm not convinced my spec totally solves this either, as hiding a popover hides child popovers, which would include the new ones added, and they'd be hidden with events. Maybe.

Comment thread source Outdated
Comment thread source Outdated
Comment thread source
<var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>
<p>To <dfn>hide popover stack until</dfn>, given an <span data-x="html elements">HTML
element</span> or null <var>endpoint</var>, <code>Document</code> <var>document</var>,
an <span data-x="attr-popover-auto-state">Auto</span> or <span data-x="attr-popover-hint-state">Hint</span> <var>stackType</var>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we have precedent for using attribute state as a parameter and it's probably reasonable as it's the result of a computation so doesn't depend on any other state.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was unsure about this. I don't mind doing something else.

Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
data-x="attr-popover-auto-state">Auto</span> state or <var>endpoint</var>'s <code
data-x="attr-popover">popover</code> attribute is in the <span
data-x="attr-popover-hint-state">Hint</span> state.</p></li>
<li><p>Reverse <var>toHide</var>.</p></li>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverse also needs an issue or link if it already exists.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to self: When I incorporate this, it returns a new list rather than mutating the current list.

Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
@jakearchibald
Copy link
Copy Markdown
Collaborator Author

@mfreed7 can you create a PR for your tests so far?

Comment thread source
<span>"<code>InvalidStateError</code>"</span> <code>DOMException</code>.</p></li>

<li><p>Return false.</p></li>
</ol>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread source
Comment on lines +91689 to +91691
<p>To <dfn>hide popovers until</dfn>, given an <span data-x="html elements">HTML
element</span> or null <var>endpoint</var>, <code>Document</code> <var>document</var>,
a boolean <var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to switch these arguments around, as then it provides a clear signal to implementers that this can be a method on Document.

Suggested change
<p>To <dfn>hide popovers until</dfn>, given an <span data-x="html elements">HTML
element</span> or null <var>endpoint</var>, <code>Document</code> <var>document</var>,
a boolean <var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>
<p>To <dfn>hide popovers until</dfn>, given <code>Document</code> <var>document</var>, an <span data-x="html elements">HTML
element</span> or null <var>endpoint</var>,
a boolean <var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>

Comment thread source
<li><p>Run <span>hide popover stack until</span> given <var>endpoint</var>, <var>document</var>'s
<span>showing auto popover list</span>, <var>focusPreviousElement</var>, and
<var>fireEvents</var>.</p></li>
<li><p>Run <span>hide popover stack until</span> given <var>endpoint</var>, <var>document</var>, <span data-x="attr-popover-auto-state">Auto</span>, <var>focusPreviousElement</var>, and <var>fireEvents</var>.</p></li>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here WRT flipping args:

Suggested change
<li><p>Run <span>hide popover stack until</span> given <var>endpoint</var>, <var>document</var>, <span data-x="attr-popover-auto-state">Auto</span>, <var>focusPreviousElement</var>, and <var>fireEvents</var>.</p></li>
<li><p>Run <span>hide popover stack until</span> given <var>document</var>, <var>endpoint</var>, <span data-x="attr-popover-auto-state">Auto</span>, <var>focusPreviousElement</var>, and <var>fireEvents</var>.</p></li>

Comment thread source
data-x="attr-popover-none-state">No Popover</span> state, then set <var>element</var>'s
<span>previously focused element</span> to <var>originallyFocusedElement</var>.</p></li>

<li><p>If <var>originalType</var> is <span data-x="attr-popover-hint-state">Hint</span> and <var>ancestor</var>'s <span>opened in popover mode</span> is "<code data-x="">auto</code>", then set <var>document</var>'s <span>hint stack parent</span> to <var>ancestor</var>.</p></li>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need a checkAncestor here due to focus events...

Or we can move this above "Run the popover focusing steps given element"

annevk pushed a commit to whatwg/infra that referenced this pull request Apr 14, 2026
For whatwg/html#12345.

This also changes the definition of clone, since it's just a slice with default arguments.
Comment thread source
Comment on lines +91536 to +91538
<li><p>Let <var>autoPopoverListContainsElement</var> be true if <var>document</var>'s <span>showing auto popover list</span> <span data-x="list contains">contains</span> <var>element</var>.</p></li>

<li><p>Let <var>hintPopoverListContainsElement</var> be true if <var>document</var>'s <span>showing hint popover list</span> <span data-x="list contains">contains</span> <var>element</var>.</p></li>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add 'otherwise false'.

@mfreed7
Copy link
Copy Markdown
Collaborator

mfreed7 commented Apr 14, 2026

@mfreed7 can you create a PR for your tests so far?

Yes. Our infra used to file a PR as soon as a Chromium CL went to the review-requested state, but it apparently no longer does that. Since I'm still waiting on the review, the new tests are located here for now:

https://chromium-review.git.corp.google.com/c/chromium/src/+/7727959

@jakearchibald
Copy link
Copy Markdown
Collaborator Author

@mfreed7 cool I'll give them a full review when there's a wpt PR. Could you review this PR?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 15, 2026
See this discussion for more context:

whatwg/html#12304
whatwg/html#12345

This CL implements a simplified behavioral model for popover=hint and
popover=auto, cleanly separating the autoRootedPopoverStack from the
hintRootedPopoverStack and introducing a `hintStackParent` to track
where the hint stack branches off the auto stack.

The new behavior resolves the following inconsistencies (gated behind
the `PopoverHintNewBehavior` experimental runtime flag):

1. Opening a hint popover will not hide unrelated auto popovers.
2. Opening a hint popover closes only other non-ancestor hint
   popovers.
3. Clicking outside consistently closes both auto and hint popovers.
4. Hiding an auto popover closes only its child popovers.
5. Opening an auto popover inside a hint popover is disallowed and
   will fail.

A new WPT test (`popover-hint-hierarchy.html`) is added to explicitly
assert these 5 rules, and existing popover WPTs are updated to reflect
the new behavior. A virtual test suite (`popover-hint-old-behavior`)
is also added to continue testing the legacy behavior. I'm not sure
how to best handle the changed tests (and I guess the new test too)
in the context of a spec PR that is forthcoming. I'd like to be able
to publish (as a WPT PR at least) the changes, to help with the PR
effort. Suggestions appreciated.

A note about the diff: I made all of the feature flag checks look like
`if (!feature enabled) {` in the hopes that the old code would show
as unchanged in the diff, but gerrit's diff algorithm isn't great.
It's at least a little better like this than if I put the new code
first, so I left it.

Bug: 4990199
Change-Id: I41d8d96be71ba60ec5e7aa640c935258d1e33431
@mfreed7
Copy link
Copy Markdown
Collaborator

mfreed7 commented Apr 15, 2026

@mfreed7 cool I'll give them a full review when there's a wpt PR. Could you review this PR?

web-platform-tests/wpt#59237

And yes! I've been swamped but I'll give the PR a look right now.

beckysiegel pushed a commit to chromium/chromium that referenced this pull request Apr 15, 2026
See this discussion for more context:

whatwg/html#12304
whatwg/html#12345

This CL implements a simplified behavioral model for popover=hint and
popover=auto, cleanly separating the autoRootedPopoverStack from the
hintRootedPopoverStack and introducing a `hintStackParent` to track
where the hint stack branches off the auto stack.

The new behavior resolves the following inconsistencies (gated behind
the `PopoverHintNewBehavior` experimental runtime flag):

1. Opening a hint popover will not hide unrelated auto popovers.
2. Opening a hint popover closes only other non-ancestor hint
   popovers.
3. Clicking outside consistently closes both auto and hint popovers.
4. Hiding an auto popover closes only its child popovers.
5. Opening an auto popover inside a hint popover is disallowed and
   will fail.

A new WPT test (`popover-hint-hierarchy.html`) is added to explicitly
assert these 5 rules, and existing popover WPTs are updated to reflect
the new behavior. A virtual test suite (`popover-hint-old-behavior`)
is also added to continue testing the legacy behavior. I'm not sure
how to best handle the changed tests (and I guess the new test too)
in the context of a spec PR that is forthcoming. I'd like to be able
to publish (as a WPT PR at least) the changes, to help with the PR
effort. Suggestions appreciated.

A note about the diff: I made all of the feature flag checks look like
`if (!feature enabled) {` in the hopes that the old code would show
as unchanged in the diff, but gerrit's diff algorithm isn't great.
It's at least a little better like this than if I put the new code
first, so I left it.

Bug: 499019927
Change-Id: I41d8d96be71ba60ec5e7aa640c935258d1e33431
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7727959
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1615186}
Copy link
Copy Markdown
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorially this looks fine now, thanks for the Infra updates! Please let me know when OP is complete, tests have been reviewed, and at least one person involved did a comprehensive technical review.

Comment thread source
<dfn data-x="list sort descending" data-x-href="https://infra.spec.whatwg.org/#list-sort-in-descending-order">sort in descending order</dfn>
<dfn data-x="list slice" data-x-href="https://infra.spec.whatwg.org/#list-slice">slice</dfn> (and its
<dfn data-x="list slice from" data-x-href="https://infra.spec.whatwg.org/#list-slice-from">from</dfn> and
<dfn data-x="list slice to" data-x-href="https://infra.spec.whatwg.org/#list-slice-to">to</dfn> options), and
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<dfn data-x="list slice to" data-x-href="https://infra.spec.whatwg.org/#list-slice-to">to</dfn> options), and
<dfn data-x="list slice to" data-x-href="https://infra.spec.whatwg.org/#list-slice-to">to</dfn> arguments), and

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 15, 2026
See this discussion for more context:

whatwg/html#12304
whatwg/html#12345

This CL implements a simplified behavioral model for popover=hint and
popover=auto, cleanly separating the autoRootedPopoverStack from the
hintRootedPopoverStack and introducing a `hintStackParent` to track
where the hint stack branches off the auto stack.

The new behavior resolves the following inconsistencies (gated behind
the `PopoverHintNewBehavior` experimental runtime flag):

1. Opening a hint popover will not hide unrelated auto popovers.
2. Opening a hint popover closes only other non-ancestor hint
   popovers.
3. Clicking outside consistently closes both auto and hint popovers.
4. Hiding an auto popover closes only its child popovers.
5. Opening an auto popover inside a hint popover is disallowed and
   will fail.

A new WPT test (`popover-hint-hierarchy.html`) is added to explicitly
assert these 5 rules, and existing popover WPTs are updated to reflect
the new behavior. A virtual test suite (`popover-hint-old-behavior`)
is also added to continue testing the legacy behavior. I'm not sure
how to best handle the changed tests (and I guess the new test too)
in the context of a spec PR that is forthcoming. I'd like to be able
to publish (as a WPT PR at least) the changes, to help with the PR
effort. Suggestions appreciated.

A note about the diff: I made all of the feature flag checks look like
`if (!feature enabled) {` in the hopes that the old code would show
as unchanged in the diff, but gerrit's diff algorithm isn't great.
It's at least a little better like this than if I put the new code
first, so I left it.

Bug: 4990199
Change-Id: I41d8d96be71ba60ec5e7aa640c935258d1e33431
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7727959
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1615186}
Copy link
Copy Markdown
Collaborator

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good to me. Thanks again for putting it together.

Comment thread source
false, <var>throwExceptions</var>, and null is false, then return.</p></li>

<li>
<p>Let <var>checkAncestor</var> be the following steps:</p>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are inline algorithms like this camelCase? (I know we already have cleanupShowingFlag here, but it just looked funny right next to "running check popover validity".)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables are always camelCase, regardless of what they hold.

Something ending in Flag sounds wrong, especially in new prose.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok, thanks. The cleanupShowingFlag was here before, but I guess now's a great time to rename it?

Comment thread source
construction of a well-formed tree from the (possibly cyclic) graph of connections. Only <span
data-x="attr-popover-auto-state">Auto</span> popovers are considered.</p>
data-x="attr-popover-auto-state">Auto</span> and <span
data-x="attr-popover-hint-state">Hint</span> popovers are considered.</p>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix.

Comment thread source
<ol>
<li><p>Run <span>close entire popover list</span> given <var>document</var>'s <span>showing
hint popover list</span>, <var>focusPreviousElement</var>, and <var>fireEvents</var>.</p></li>
<li><p>If <var>endpointIsHint</var>, then return.</p></li>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with this behavior, if you have an Auto popover (A1), which opens a Hint popover (H1). You also have another Auto popover (A2) opened higher/later in the auto stack. The user clicks inside H1. I thought (and implemented) that in this case, A2 should be closed, because A1 is H1's hint stack parent. But this algorithm won't do that. Is that intentional? Your proposal says "Proposed: All auto/hint popovers are closed except any clicked popover and its parents."

Comment thread source

<li>
<p>Perform the following steps at least once:</p>
<li><p>Let <var>lastHideIndex</var> be 0 if <var>popoverList</var> does not <span data-x="list contains">contain</span> <var>endpoint</var>; otherwise the index of <var>endpoint</var> in <var>popoverList</var> plus 1.</p></li>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in reading this, I think I caught a bug in my Chromium impl, which might mean I also wrote the corresponding bug into the WPTs. I'll take a look there, but you might find it also as you review, so FYI.

Comment thread source

<li><p>Let <var>candidatePosition</var> be
<var>popoverPositions</var>[<var>candidateAncestor</var>].</p></li>
<li><p>Let <var>ancestorIndex</var> be the minimum of <var>popoverAncestorIndex</var> and <var>sourceAncestorIndex</var>.</p></li>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think perhaps this should be the maximum not the minimum? Scenario:

<div id=A1 popover=auto>A1
  <button id=buttonA2 popovertarget=A2>Open A2</button>
  <div id=A3 popover=auto>A3</div>
</div>
<div id=A2 popover=auto>A2
  <button id=buttonA3 popovertarget=A3>Open A3</button>
</div>
<script>
A1.showPopover();
buttonA2.click();
buttonA3.click(); // <-- difference here
</script>

When A2 is opened, A1 is its ancestor (via popovertarget=A2). When A3 is opened, it can either have A1 as its ancestor (via DOM) or A2 as its ancestor (via popovertarget=A3). I think the reference higher up in the stack should probably win here. As written, the spec will close A2, even though its button opened A3.

{When I fix the Chromium bug above, I'll be sure to add a test case like this to a WPT.}

Comment thread source
<p>To <dfn export data-x="hide-all-popovers-until">hide all popovers until</dfn>, given an <span
data-x="HTML elements">HTML element</span> or <code>Document</code> <var>endpoint</var>, a boolean
<var>focusPreviousElement</var>, and a boolean <var>fireEvents</var>:</p>
<p>To <dfn>hide popover stack until</dfn>, given an <span data-x="html elements">HTML
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and sorry.

Comment thread source

<p class="note">These checks are performed again because running <span>hide popover stack until</span> above could have fired the <code data-x="event-beforetoggle">beforetoggle</code> event, and an event handler could have altered DOM state.</p>
</li>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible there's a missing case here too. The beforetoggle could have opened a fresh hint, and this code will allow it to stay on the hint stack while still opening the new (unrelated) auto. Example:

<div id=A1 popover=auto>A1</div>
<div id=A2 popover=auto>A2</div>
<div id=H1 popover=hint>H1</div>
<script>
A1.showPopover();
A1.onbeforetoggle = () => H1.showPopover();
A2.showPopover();
</script>

I'll add a WPT for this too. I think the fix it maybe to just check right here that the hint stack is still empty before proceeding?

@mfreed7
Copy link
Copy Markdown
Collaborator

mfreed7 commented Apr 15, 2026

The new tests I referred to above are here now: https://crrev.com/c/7765209. I'll comment again when those become a WPT PR.

beckysiegel pushed a commit to chromium/chromium that referenced this pull request Apr 16, 2026
This fixes an error in the recently-landed popover=hint behavior at
[1], for this case:

Auto popover (A1) is the anchor for a Hint popover (H1). Another Auto
popover (A2) is opened on top. The Auto stack is [A1, A2] and the
hint_parent is A1.  While hiding all popovers down to A1, the old code
would hide H1, even though it left H1's anchor (A1) open.

It also adds two tests for cases discovered during the review of the
spec PR [2]. Those were correct in Chromium, but incorrect in the
spec.

[1] https://crrev.com/c/7727959
[2] whatwg/html#12345

Bug: 499019927
Change-Id: I917bf939614f9811470444e9f13c942e5c63b1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7765209
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1616163}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

popover=hint has multiple weird and inconsistent behaviours

5 participants