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

[inert] Add WPTs for interaction of inert attribute with modal dialog #31668

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Nov 18, 2021

https://html.spec.whatwg.org/multipage/interaction.html#inert is clear:

While document is so blocked [by a modal dialog], every node that is
connected to document, with the exception of the subject element and
its shadow-including descendants, must be marked inert. (The elements
excepted by this paragraph can additionally be marked inert through
other means; being part of a modal dialog does not "protect" a node
from being marked inert.)

However, both Firefox and WebKit ignore this, and let modal dialogs
escape the inertness set by an ancestor with the 'inert' attribute.

There is also an interesting case: when the modal dialog is the root
element. Firefox handles this fine, but Chromium used to fail the test,
and WebKit directly crashes.

Bug: 692360
Change-Id: Ie52201a1f31790392180c96a9e08be1b5eee86d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3290703
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#945411}

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.

@nt1m
Copy link
Member

nt1m commented Nov 19, 2021

@Loirooriol I don't think the specs are necessarily clear on this.

https://fullscreen.spec.whatwg.org/#new-stacking-layer says:

Each element and ::backdrop pseudo-element in a top layer has the following characteristics:

  • It is rendered as an atomic unit as if it were a sibling of its root.

Following this definition, modal dialogs should escape inertness, given inert is implemented based on style/rendering.

From a web perspective, I don't think anyone will try to open a fullscreen element/modal dialog, with the intent for them to be inert. And if it is their intent to do so, they can set inert explicitly on the modal element.

w3c/csswg-drafts#6685 (comment) also resolved on WK behavior fwiw which I described explicitly in the previous comment with the "modal element" proposal being described.

cc @emilio

@nt1m
Copy link
Member

nt1m commented Nov 19, 2021

The HTML spec has yet to be updated: whatwg/html#7134

@Loirooriol
Copy link
Contributor

@nt1m I don't think the CSSWG resolved about dialogs:

So punt on explicitly-inert dialog question for now

Also, seems potentially out of scope for CSS, and better addressed by the WHATWG.

It is rendered as an atomic unit as if it were a sibling of its root.

To me this doesn't necessarily imply that it escapes inertness. The CSS cascade still treats it as a child of its parent, not a child of the root.

Like, I'm fine with switching Chromium to what Firefox and WebKit do, it would actually be simpler since inertness could be stored as a single bool instead of a 3-valued enum. But the HTML spec seems pretty clear to me, even with your PR whatwg/html#7134

Anyways, regardless of what we do, we need tests. Especially since the 2nd one crashes WebKit.

@mfreed7
Copy link
Contributor

mfreed7 commented Nov 24, 2021

To me this doesn't necessarily imply that it escapes inertness. The CSS cascade still treats it as a child of its parent, not a child of the root.

I just wanted to say that I agree with this part. CSS selects <dialog> exactly as if it is in the DOM tree, and not a child of <html>. I wouldn't think inert should be any different, should it? I.e. try this example:

<div style="display:none">
  <dialog id=d>This is a test</dialog>
</div>
<script>
  window.onload = () => d.showModal();
</script>

Do you think the <dialog> should also escape the display:none?

@nt1m
Copy link
Member

nt1m commented Nov 24, 2021

@mfreed7 It's quite unclear tbh, the top layer escapes lots of things. It escapes ancestor CSS opacity/mask for instance according to the spec:

It is rendered as an atomic unit as if it were a sibling of its root.
Ancestor elements with overflow, opacity, masks, etc. cannot affect it.

I think this should be clarified when the top layer is moved to a CSS spec. The "etc." is vague and I'm not quite sure what it covers. For instance, if it covers pointer-events/user-select, then it probably makes sense to escape inert as well.

Also, my second argument would be that this is similar to setting this CSS:

html {
  pointer-events: none;
}

dialog[open]:not([inert]) {
  pointer-events: auto;
}

This is slightly easier to understand in CSS terms. And this reflects in the implementation too, this required a tri-state value for taking in account the ancestor.

I don't feel too strongly about this tbh, and I think Emilio doesn't either...

https://html.spec.whatwg.org/multipage/interaction.html#inert is clear:

> While document is so blocked [by a modal dialog], every node that is
> connected to document, with the exception of the subject element and
> its shadow-including descendants, must be marked inert. (The elements
> excepted by this paragraph can additionally be marked inert through
> other means; being part of a modal dialog does not "protect" a node
> from being marked inert.)

However, both Firefox and WebKit ignore this, and let modal dialogs
escape the inertness set by an ancestor with the 'inert' attribute.

There is also an interesting case: when the modal dialog is the root
element. Firefox handles this fine, but Chromium used to fail the test,
and WebKit directly crashes.

Bug: 692360
Change-Id: Ie52201a1f31790392180c96a9e08be1b5eee86d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3290703
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#945411}
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.

6 participants