Close unrelated auto popovers when a dialog opens inside a hint popover - #12764
Conversation
The dialog show() and show a modal dialog algorithms call hide popovers until with the dialog's topmost popover ancestor. When that ancestor was a hint popover, the algorithm returned after hiding the hint stack and left every auto popover open. Light dismiss already handled this case: it hides auto popovers up to the document's hint stack parent, so the auto popovers the hint hangs off stay open and the rest close. Adopt that behaviour in hide popovers until, and have light dismiss call it instead of duplicating the two hide popover stack until calls.
There was a problem hiding this comment.
Pull request overview
This PR updates the HTML popover dismissal algorithms so that when a dialog opens from within a hint popover, unrelated auto popovers are correctly closed (matching the behavior previously implemented in light dismiss), aligning spec behavior with the linked WPT expectations for #12763.
Changes:
- Update hide popovers until to close auto popovers even when the endpoint is a hint popover, using the document’s hint stack parent as the auto-stack endpoint.
- Refactor the light-dismiss pointer handling to reuse hide popovers until instead of duplicating the hide-stack logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note that "hide popovers until" is exported and also called by the fullscreen API. I think this behaviour makes sense there too. I'll add a test. |
|
This changing the behaviour for non-modal dialogs is perhaps a bit weird as they're in many ways like a manual popover. |
|
Happy to discuss this at WHATNOT. |
|
Could folks review the tests? web-platform-tests/wpt#61795 - even if we tweak the behaviours it's good to know these look right, especially as it seems like the current tests were wrong, even before the spec change. |
I agree it's weird, because they're not in the top layer like the rest. Also, I think they maybe shouldn't exist anyway. So maybe it's "ok" to change their behavior here? |
|
@keithamus Anne is currently out on vacation. Maybe @zcorpan could take a look? |
|
I'll take @mfreed7 's review as satisfying implementer interest for the OP. @jakearchibald can you fill in the implementation bugs? |
|
@zcorpan done! |
…en, and remove redundant test See whatwg/html#12764
…pens r=dom-core-reviewers,smaug This updates these algorithms to match the spec changes proposed in whatwg/html#12764 Differential Revision: https://phabricator.services.mozilla.com/D317541
…pens r=dom-core-reviewers,smaug This updates these algorithms to match the spec changes proposed in whatwg/html#12764 Differential Revision: https://phabricator.services.mozilla.com/D317541
This follows the html spec and WPT PRs: whatwg/html#12764 web-platform-tests/wpt#61795 This change moves the logic that closes popover=hint stacks out of light dismiss handling and directly into HideAllPopoversUntil, and tweaks it slightly w.r.t. the hint stack. This handles a corner case where top-layer elements (such as dialogs or fullscreen elements) nested within popovers need to hide hint popovers correctly without relying on light-dismiss-specific code paths. Bug: 499019927 Fixed: 544919867,543985662,545676304 Change-Id: I1bd5f58ce491ccab5698a54a6033f28d37ffca48 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8223345 Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1679562}
Commit message:
Close unrelated auto popovers when a dialog opens inside a hint popover
The algorithm the show-dialog methods were using was wrong. Light dismiss had the algorithm right. I've updated "hide popovers until" to use the light dismiss behaviour, and now light dismiss uses the "hide popovers until".
Fixes #12763.
(See WHATWG Working Mode: Changes for more details.)
/popover.html ( diff )