Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow top layer elements to be nested within popovers #44146

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 23, 2024

See the issue raised at

whatwg/html#9998

which was discussed at

whatwg/html#9993

This CL makes the following changes:

  1. Change FindTopmostPopoverAncestor() so that the provided element
    does not have to be a popover. The logic does not materially
    change - all of the same mechanisms can be used to connect a
    non-popover top layer element (dialog or fullscreen) to an ancestor
    popover.
  2. Add a utility TopLayerElementPopoverAncestor() which finds the
    popover ancestor for a provided top layer element by calling
    FindTopmostPopoverAncestor() with the proper arguments.
  3. In dialog and fullscreen code, where it previously called
    HideAllPopoversUntil(nullptr,...) to hide all open popovers,
    it now (with flag enabled) hides only up to the nearest popover
    ancestor.

Tests are added, which are marked .tentative.

Bug: 1520938
Change-Id: I8d2c4000ed3959ac4e8bf521e22e9dfd532c62d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5229300
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1254541}

See the issue raised at

  whatwg/html#9998

which was discussed at

  whatwg/html#9993

This CL makes the following changes:
 1. Change `FindTopmostPopoverAncestor()` so that the provided element
    does not have to be a popover. The logic does not materially
    change - all of the same mechanisms can be used to connect a
    non-popover top layer element (dialog or fullscreen) to an ancestor
    popover.
 2. Add a utility `TopLayerElementPopoverAncestor()` which finds the
    popover ancestor for a provided top layer element by calling
    `FindTopmostPopoverAncestor()` with the proper arguments.
 3. In dialog and fullscreen code, where it previously called
    `HideAllPopoversUntil(nullptr,...)` to hide **all** open popovers,
    it now (with flag enabled) hides only up to the nearest popover
    ancestor.

Tests are added, which are marked `.tentative`.

Bug: 1520938
Change-Id: I8d2c4000ed3959ac4e8bf521e22e9dfd532c62d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5229300
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1254541}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit a8a4f24 into master Jan 31, 2024
17 of 19 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-5229300 branch January 31, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants