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

Opening a dialog as modal, nested inside a popover, causes the whole page to go inert with limited ways for the user to address it. #9998

Closed
keithamus opened this issue Dec 15, 2023 · 3 comments · Fixed by #10116
Labels
topic: dialog The <dialog> element topic: popover The popover attribute and friends

Comments

@keithamus
Copy link
Contributor

keithamus commented Dec 15, 2023

What is the issue with the HTML Standard?

Given some markup like:

<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>  
<button>I do nothing!</button>

With some JS like

openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

Some repro steps:

  1. Visit https://codepen.io/keithamus/pen/wvNLKeV (in a browser that supports native popover)
  2. Click "Open Popover"
  3. Click "Open Dialog"
  4. Observe no visible top layer elements, the page is inert and seems to no longer be interactive.

This issue seems to stem from the fact that opening a dialog will hide all open popovers, and due to the nesting this - for reasons that are not evident to me - hide the dialog. Given the dialog is modal, and therefore on the top layer, it seems to me that the dialog should remain visible.

This is an issue across 3 browsers - Chrome, Safari, and Firefox (with dom.element.popover.enabled to true). Therefore I think it's an issue with how the feature is specified, though perhaps this is more for the CSSWG?

I've discussed this a little already in the OpenUI CG with @josepharhar and @lukewarlow.

@keithamus keithamus added topic: dialog The <dialog> element agenda+ To be discussed at a triage meeting topic: popover The popover attribute and friends labels Dec 15, 2023
@keithamus keithamus changed the title Opening a dialog as modal, nested inside a popover, causes the whole page to go inert with no way to fix it Opening a dialog as modal, nested inside a popover, causes the whole page to go inert with limited ways for the user to address it. Dec 15, 2023
@domenic
Copy link
Member

domenic commented Dec 16, 2023

Maybe a dupe of #9936 in root causes, although I guess this describes the reverse situation to that bug.

@past past removed the agenda+ To be discussed at a triage meeting label Jan 11, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 24, 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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 30, 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
@josepharhar
Copy link
Contributor

I opened a PR for this: #10116

josepharhar added a commit to josepharhar/fullscreen that referenced this issue Jan 31, 2024
Without this patch, the fullscreening an element inside an open popover
will make the fullscreen element display:none.

Issue: whatwg/html#9998
Corresponding HTML PR: whatwg/html#10116
@josepharhar
Copy link
Contributor

And another one for fullscreen: whatwg/fullscreen#237

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 31, 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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 31, 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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 31, 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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 2, 2024
…thin popovers, a=testonly

Automatic update from web-platform-tests
Allow top layer elements to be nested within popovers

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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}

--

wpt-commits: a8a4f241414cdabb1854db1aef3493446bbddddf
wpt-pr: 44146
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Feb 2, 2024
…thin popovers, a=testonly

Automatic update from web-platform-tests
Allow top layer elements to be nested within popovers

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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}

--

wpt-commits: a8a4f241414cdabb1854db1aef3493446bbddddf
wpt-pr: 44146
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 6, 2024
…ent. r=smaug

Given some markup like:

```html
<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>
<button>I do nothing!</button>
```

With some JS like

```js
openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}
```

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape).

This was reported in whatwg/html#9998, and WhatWG resolved to fix this, which in whatwg/html#10116.

Differential Revision: https://phabricator.services.mozilla.com/D200686
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Feb 7, 2024
…ent. r=smaug

Given some markup like:

```html
<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>
<button>I do nothing!</button>
```

With some JS like

```js
openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}
```

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape).

This was reported in whatwg/html#9998, and WhatWG resolved to fix this, which in whatwg/html#10116.

Differential Revision: https://phabricator.services.mozilla.com/D200686
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 8, 2024
…thin popovers, a=testonly

Automatic update from web-platform-tests
Allow top layer elements to be nested within popovers

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 <masonfchromium.org>
Commit-Queue: Joey Arhar <jarharchromium.org>
Reviewed-by: Joey Arhar <jarharchromium.org>
Cr-Commit-Position: refs/heads/main{#1254541}

--

wpt-commits: a8a4f241414cdabb1854db1aef3493446bbddddf
wpt-pr: 44146

UltraBlame original commit: e7982e55cf91f90fccd5f34682858cc6cf1c11eb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 8, 2024
…ent. r=smaug

Given some markup like:

```html
<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>
<button>I do nothing!</button>
```

With some JS like

```js
openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}
```

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape).

This was reported in whatwg/html#9998, and WhatWG resolved to fix this, which in whatwg/html#10116.

Differential Revision: https://phabricator.services.mozilla.com/D200686

UltraBlame original commit: 52284e30cea2c52ab073310e4010161702dc92e4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 8, 2024
…thin popovers, a=testonly

Automatic update from web-platform-tests
Allow top layer elements to be nested within popovers

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 <masonfchromium.org>
Commit-Queue: Joey Arhar <jarharchromium.org>
Reviewed-by: Joey Arhar <jarharchromium.org>
Cr-Commit-Position: refs/heads/main{#1254541}

--

wpt-commits: a8a4f241414cdabb1854db1aef3493446bbddddf
wpt-pr: 44146

UltraBlame original commit: e7982e55cf91f90fccd5f34682858cc6cf1c11eb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 8, 2024
…ent. r=smaug

Given some markup like:

```html
<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>
<button>I do nothing!</button>
```

With some JS like

```js
openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}
```

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape).

This was reported in whatwg/html#9998, and WhatWG resolved to fix this, which in whatwg/html#10116.

Differential Revision: https://phabricator.services.mozilla.com/D200686

UltraBlame original commit: 52284e30cea2c52ab073310e4010161702dc92e4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Feb 8, 2024
…thin popovers, a=testonly

Automatic update from web-platform-tests
Allow top layer elements to be nested within popovers

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 <masonfchromium.org>
Commit-Queue: Joey Arhar <jarharchromium.org>
Reviewed-by: Joey Arhar <jarharchromium.org>
Cr-Commit-Position: refs/heads/main{#1254541}

--

wpt-commits: a8a4f241414cdabb1854db1aef3493446bbddddf
wpt-pr: 44146

UltraBlame original commit: e7982e55cf91f90fccd5f34682858cc6cf1c11eb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Feb 8, 2024
…ent. r=smaug

Given some markup like:

```html
<div id=popover popover>
  <button id="openDialog">Open Dialog</button>
  <dialog id=dialog>
    I'm a dialog!
  </dialog>
</div>
<button id="openPopover">Open Popover</button>
<button>I do nothing!</button>
```

With some JS like

```js
openDialog.onclick = () => {
  dialog.showModal();
}

openPopover.onclick = () => {
  popover.showPopover();
}
```

Clicking the "Open Popover" button followed by the "Open Dialog" button results in both the Dialog and Popover being hidden, however the dialog will still be open as modal, rendering the whole page inert, nothing is clickable and there seems to be no way to undo this (unless you use a CloseWatcher gesture such as the Esc key - if you have one available on your device).

It is expected that the popover should not close the dialog, as it causes the invisible Modal Dialog to make the whole page inert, and it is very difficult for users (and developers) to discover how to undo this action (pressing escape).

This was reported in whatwg/html#9998, and WhatWG resolved to fix this, which in whatwg/html#10116.

Differential Revision: https://phabricator.services.mozilla.com/D200686

UltraBlame original commit: 52284e30cea2c52ab073310e4010161702dc92e4
mbrodesser-Igalia pushed a commit to mbrodesser-Igalia/wpt that referenced this issue Feb 19, 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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Feb 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 <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1254541}
domenic pushed a commit that referenced this issue Mar 26, 2024
This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes #9998. See also whatwg/fullscreen#237.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Apr 8, 2024
This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Apr 8, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Apr 8, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
domenic pushed a commit to whatwg/fullscreen that referenced this issue May 7, 2024
Without this change, fullscreening an element inside an open popover will make the fullscreen element display:none.

Issue: whatwg/html#9998
Corresponding HTML PR: whatwg/html#10116
rubberyuzu added a commit to rubberyuzu/html that referenced this issue May 13, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue May 20, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Jun 17, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Jun 25, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Aug 20, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
rubberyuzu added a commit to rubberyuzu/html that referenced this issue Aug 27, 2024
Allow top layer elements to be nested within popovers

This allows top layer elements, including the dialog element, to be nested inside of an open popover, by not closing the popover when the new top layer element is opened. Without this patch, opening a modal dialog inside of a popover will make the page inert and make the dialog invisible.

Fixes whatwg#9998. See also whatwg/fullscreen#237.

Editorial: order of comparisons

For consistency:
- greater than or equal to
- less than or equal to

Improve element reflection

This attempts to make the following improvements:

1. Make it more clear that initializing attr is not the first step in an algorithm, but rather something that counts for all the list items.
2. Rewrite the associated element(s) fields as algorithms. As there are no downstream references so far this is a change we can still make.
3. Add another layer of caching that is separate from the FrozenArray to avoid having to compare a list of elements with a FrozenArray directly.

This helps with whatwg#10219.

Disable PageSwapEvent's activation on cross-origin redirects

Closes whatwg#10196.

Upstream Long Animation Frames monkey-patches

Long Animation Frames (https://w3c.github.io/long-animation-frames/) expects a few calls from HTML and other specs, for reporting when tasks, rendering or JS entry points take place. This moves those calls from the Long Animation Frames spec to HTML.

Preload: only allow certain values for as=""

Closes whatwg#8332.

Call the view transition page visibility change steps

This allows the CSS view-transitions spec to react to page visibility changes. Specifically, skip the active transition once a page is hidden.

See w3c/csswg-drafts#9543.

Style marquee using overflow: hidden

This matches Chromium and WebKit. Tests will be worked on in https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Editorial: export Element's innerText getter and setter steps

These will be used by Trusted Types (and eventually HTML once upstreamed) as part of shadowing this attribute to HTMLScriptElement.

Add getHTML() and serializable shadow roots

Corresponding DOM PR: whatwg/dom#1256.

Closes whatwg#8867.

Co-authored-by: Domenic Denicola <[email protected]>

Make buttons respect display: none/contents in button layout

Fixes whatwg#10238. This matches what is already implemented in browsers.

Remove duplicate requirement for 'overflow' for marquee

The duplication was introduced by whatwg#10243.

Meta: make all the SVGs darkmode-aware

Also tag them as such, so that they don't get a white background after whatwg/whatwg.org#439 is merged.

Warn that the XML syntax is not recommended

Closes whatwg#10237.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: dialog The <dialog> element topic: popover The popover attribute and friends
Development

Successfully merging a pull request may close this issue.

4 participants