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: Hit-testing / selection behavior shouldn't be magical. #5650

Closed
emilio opened this issue Jun 17, 2020 · 17 comments
Closed

inert: Hit-testing / selection behavior shouldn't be magical. #5650

emilio opened this issue Jun 17, 2020 · 17 comments

Comments

@emilio
Copy link
Contributor

emilio commented Jun 17, 2020

The inert attribute as defined in #4288 changes hit testing and selection behavior in ways that can already be controlled via CSS via pointer-events and user-select, respectively. It also forces cursor to be default, in practice.

It seems to me that, rather than adding a second magic way of controlling this behavior, defining this in terms of either existing or new values for those CSS properties makes more sense. Defining inert in a way that inert nodes (note: not nodes with the inert content attribute) must have a computed value of pointer-events: none and user-select: none is the easiest way to address this.

This would be equivalent to the following pseudo-code in a user-agent stylesheet:

/* matches all inert elements. Could be an internal pseudo-class, or may be useful to expose it for authors? Not sure */
:inert {
  user-select: none !important;
  pointer-events: none !important;
  cursor: default !important;
}

Per the discussion in mozilla/standards-positions#174, there's a subtle difference between the behavior of pointer-events: none and the intended behavior of inert, which is that pointer-events: none doesn't prevent boxes behind (but not inside) the inert node from getting hit. If this behavior is both intentional and important, which per that discussion seems to be, then it seems such a behavior should be specifiable via pointer-events as well.

Note that focus behavior used to be controllable via CSS as well, via user-focus, but it seems it's been removed from the latest draft. cc @frivoal as editor of that spec in case he remembers why. Generally focusability already depends on CSS (visibility and overflow and such). But special-casing inert for focus if user-focus doesn't exist might be fine, as that's not something that we expose in other ways.

Also, it seems a bit odd that inert as specified changes the behavior of some selection APIs, like setSelectionRange, but not selectionStart / selectionEnd setters. Why is that needed / desirable?

Generally, I don't think inert should be described in terms of event dispatching / selection magic when we have existing features in the platform to control what inert wants to control.

cc: @annevk @smaug---- @alice @bkardell @robdodson @smfr

@dbaron
Copy link
Member

dbaron commented Jun 18, 2020

It's not clear to me that the inert behavior fits well with the pointer-events property given that pointer-events is an inherited property. The behavior required for inert is something that applies to an entire subtree. While this could be defined in an inherited property, it doesn't fit particularly well, and means that you'd need to look at all ancestors' computed pointer-events before deciding that an element can be a hit target.

@alice
Copy link
Contributor

alice commented Jun 18, 2020

This would be equivalent to the following pseudo-code in a user-agent stylesheet:

This ignores the critical point that inert also adds aria-hidden-like behaviour.

Missing this point proves exactly the reason why we want to combine all these behaviours together: authors tend to miss this as well, and it creates a poor experience for users, who come first in the priority of constituencies.

Also, it seems a bit odd that inert as specified changes the behavior of some selection APIs, like setSelectionRange, but not selectionStart / selectionEnd setters. Why is that needed / desirable?

This is probably an oversight in the spec. I'm happy to fix that up.

@emilio
Copy link
Contributor Author

emilio commented Jun 18, 2020

It's not clear to me that the inert behavior fits well with the pointer-events property given that pointer-events is an inherited property. The behavior required for inert is something that applies to an entire subtree. While this could be defined in an inherited property, it doesn't fit particularly well, and means that you'd need to look at all ancestors' computed pointer-events before deciding that an element can be a hit target.

Well so is cursor (and user-select is not technically inherited but behaves almost like it). The comment in the snippet I posted specifically mentions "matches all inert elements" (which is not the same as "all elements with the inert attribute").

This ignores the critical point that inert also adds aria-hidden-like behaviour.

Sure, I was talking about the selection / hit-testing behavior. But in any case, again, I do agree inert is valuable, I just don't think we should add more switches to tweak things we already have switches for.

emilio added a commit to emilio/standards-positions that referenced this issue Jun 18, 2020
Mark it as worth-prototyping as per this comment[1] and following.

Closes #4288, let's continue the more technical discussion somewhere
else, like in whatwg/html#5650.

Same as mozilla#369, but with @dbaron's feedback addressed, because apparently
I can't re-open a PR if I have already force-pushed to the same branch.

[1]: mozilla#174 (comment)
@emilio
Copy link
Contributor Author

emilio commented Jun 18, 2020

The comment in the snippet I posted specifically mentions "matches all inert elements" (which is not the same as "all elements with the inert attribute").

Though this means that toggling "inert" on a node needs to restyle the whole subtree, which may be slightly unfortunate and one similar concern? Though you need to do work on the whole subtree with or without the CSS, to make it reasonably efficient, I think.

@alice
Copy link
Contributor

alice commented Jun 18, 2020

I do agree inert is valuable, I just don't think we should add more switches to tweak things we already have switches for.

I don't follow the logic here. Much of the value of inert is that it combines all these behaviours together, and is straightforward to use and understand: it operates on the DOM subtree, and can't be overridden.

What switches are you talking about adding?

@emilio
Copy link
Contributor Author

emilio commented Jun 18, 2020

I'm talking about adding another way to control selection / hit-testing / etc behavior that works regardless of CSS (via inert magic), when it's easy to have inert have the behavior you want, combining all these things, and allowing authors to use it easily in the same way, while basing its behavior on features that already exist, and that can be queried by authors, etc.

My proposal above achieves the "can't be overridden" bit in the same way (by using !important in the user-agent stylesheet instead of magic), while it avoids having to introduce switches in hit-testing / selection code that override the existing CSS switches, and it still allows authors to query the computed style and get meaningful results for "can this node be selected", for example.

I feel like maybe we're talking past each other a bit? I'd be happy to sync in person real quick if you wanted or what not.

@emilio
Copy link
Contributor Author

emilio commented Jun 18, 2020

Because I feel like either I'm not expressing myself correctly, or we're misunderstanding each other's propositions, or I'm missing something, or maybe something else? Again, I'm very much not against this feature, I just feel like the way it's defined and implemented could be simpler :)

@dbaron
Copy link
Member

dbaron commented Jun 18, 2020

So a few clarifications here:

  1. What @emilio is asking for is not redefining all of inert in terms of these properties (cursor, user-select, and pointer-events) but redefining part of inert in terms of them.

  2. It seems that in some cases this requires changes to those properties (I think this is needed for pointer-events and for user-select and is not needed for cursor).

This redefinition would have the advantages that:

a. It simplifies the specification of inert by deferring parts of it to CSS, and correspondingly simplifies the implementation by avoiding needing to look at the DOM for those things.

b. It improves the experience for authors who are trying to examine CSS computed values of these properties to understand what the event / selection / cursor behavior of their elements are.

c. It allows content authors who actually do want part of inert's behavior but not all of it to be able to get it. (Admittedly this may be both an advantage and a disadvantage, if they find the "part" behavior first when they really wanted "all".)

Note that (b) also means that compatibility with then-existing (but not yet existing) content might prevent us from making this redefinition in the future, which is why it's somewhat important to decide one way or another now or at least soon.

I'd also note that the required changes to user-select would I think mean reintroducing a behavior that I think Gecko used to have (although I didn't audit the history closely) where there is a none-like value that inhibits selection on all descendants even if they have a different value

@emilio
Copy link
Contributor Author

emilio commented Jun 18, 2020

Just to clarify, I don't think there's no need to change user-select, if you specify that user-select: none must apply to all inert nodes, rather than just to the root of the inert subtree, which is that my sketched pseudo-class intended to do.

@alice
Copy link
Contributor

alice commented Jun 19, 2020

There seem to be several issues conflated here:

  1. Elements which are made inert via the inert attribute (it's not clear whether this is also intended for elements which are made inert via an active modal <dialog>) should match CSS properties which have similar behaviour (e.g. user-select).

What is the user or author need for this functionality? Is there a specific case in which it would be useful? Is there a precedent for this other than hidden, which many authors find frustrating because the hidden-ness is so easy to accidentally override?

  1. We should add more mechanisms to CSS to control the things that inert is controlling (e.g. a new value for pointer-events, and possibly some iteration of user-focus).

Each of these additions should be addressing a specific user need, like inert is.

It's also not clear to me that having CSS control behaviour, particularly for things like focus, is actually a good thing. Having something be defined in HTML rather than CSS doesn't make it intrinsically "magic".

Plus, as @dbaron pointed out, giving authors more control can actually harm users, if authors actually want inert but, for example, find the relevant CSS properties first.

  1. The way inertness is specced to affect hit testing is weird.

Yes, it is. This is largely because there is no hit testing spec.

@alice
Copy link
Contributor

alice commented Jun 19, 2020

I'll also point out that Maciej Stachowiak left the following feedback on the inert repo:

The WebKit team at Apple supports the idea of implementing inert. It's not at the very top of our priority list but we're actively considering it. We'd also support putting it back in the HTML spec

Caveat: some personal opinions I haven't discussed with the team. I don't think inert would be better expressed in CSS. CSS is about presentation, not behavior. Stuff like user-select and -webkit-user-modify seems in retrospect kind of regrettable. Also, the inability to escape inert from within a subtree seems like a feature not a bug. An inert CSS property would not be sufficient for implementing dialog either, both because of the ability to escape, and because dialog also includes novel stacking behavior.

@dbaron
Copy link
Member

dbaron commented Jun 19, 2020

Is there a precedent for this other than hidden, which many authors find frustrating because the hidden-ness is so easy to accidentally override?

I think the precedent for this is in some ways a significant part of CSS1 and CSS2, which were motivated by separating out the "presentational" behaviors that existed in HTML into a separate language and thus allowing HTML to deprecate its presentational attributes for use in new documents and recommend that new documents use CSS instead and explain them in terms of their mapping into CSS. hidden is interesting because it was probably the first step in the other direction after that era, that is, the first new HTML presentational attribute in a while that was recommended to developers rather than moving presentation into CSS. But underneath that is the question of what it means to be presentational, and where the line in the separation of content versus presentation should actually be drawn. (Direct CSS control over things like selection and focus has always been one of the unclear areas, although selection and focus do clearly depend on a number of things that are firmly in on the CSS side.)

@emilio
Copy link
Contributor Author

emilio commented Jun 20, 2020

There seem to be several issues conflated here:

1. Elements which are made inert via the `inert` attribute (it's not clear whether this is also intended for elements which are made inert via an active modal `<dialog>`) should match CSS properties which have similar behaviour (e.g. `user-select`).

What is the user or author need for this functionality? Is there a specific case in which it would be useful? Is there a precedent for this other than hidden, which many authors find frustrating because the hidden-ness is so easy to accidentally override?

I think it would be useful for authors. For users there's no difference of course. Regarding other precedents, contenteditable affects the :read-write / :read-only pseudo-classes, which by default makes the cursor property be default on links and so on, at least in Gecko. This is overridable by authors right now though. hidden being easy to accidentally override seems unfortunate, but that's because of how hidden is defined, not because of hidden mapping to the display property itself.

We're clear we don't want to allow authors to override inert, but that's different from making inert cause the computed styles of user-select / etc be different.

I'm pretty sure that if we changed hidden to not map to the display property, then a lot of the code authors write would be bogus in its presence. I've seen tons of loops through the parent chain checking for display: none, and hidden not being implemented in terms of display would make those loops more complicated for authors to get right. This one is one that comes to mind, but I'm sure there's a gazillion examples. It seems reasonable for authors to query selectability or hit-testability in similar ways, and inert not mapping to the existing CSS facilities for it just makes authors' lifes harder.

1. We should add more mechanisms to CSS to control the things that `inert` is controlling (e.g. a new value for `pointer-events`, and possibly some iteration of `user-focus`).

Each of these additions should be addressing a specific user need, like inert is.

If modifying the behavior of hit testing is required for inert, I'd say that it makes sense to make it expresable via the existing switch (pointer-events).

It's also not clear to me that having CSS control behaviour, particularly for things like focus, is actually a good thing. Having something be defined in HTML rather than CSS doesn't make it intrinsically "magic".

CSS already controls focus behavior, via both display, overflow, and visibility, right? But sure, I agree the user-focus thing seems a bit more controversial. user-focus not existing today unprefixed in any browser means we probably have more leeway here and can punt on deciding whether inert should map to it whenever we introduce it, if ever.

I'll also point out that Maciej Stachowiak left the following feedback on the inert repo:

The WebKit team at Apple supports the idea of implementing inert. It's not at the very top of our priority list but we're actively considering it. We'd also support putting it back in the HTML spec

Caveat: some personal opinions I haven't discussed with the team. I don't think inert would be better expressed in CSS. CSS is about presentation, not behavior. Stuff like user-select and -webkit-user-modify seems in retrospect kind of regrettable. Also, the inability to escape inert from within a subtree seems like a feature not a bug. An inert CSS property would not be sufficient for implementing dialog either, both because of the ability to escape, and because dialog also includes novel stacking behavior.

Yeah, but that is about making inert a CSS property itself, right? Not about making it affect existing CSS properties. In any case feedback from @othermaciej / @smfr / other WebKit folks on this issue would be greatly appreciated.

@alice
Copy link
Contributor

alice commented Jun 21, 2020

Thanks for the examples, and to David for the the explanation of the philosophical precedent.

We're clear we don't want to allow authors to override inert, but that's different from making inert cause the computed styles of user-select / etc be different.

Fair point.

If modifying the behavior of hit testing is required for inert, I'd say that it makes sense to make it expresable via the existing switch (pointer-events).

I strongly disagree.

This is equivalent to exposing part of inert's behaviour in isolation, which I've already argued against repeatedly. If people want inert-like hit testing behaviour, they should use inert.

(Also, pointer-events is a weird fit for HTML in the first place, in my opinion. pointer-events: none; is very counter-intuitive in what it actually means.)

I'm pretty sure that if we changed hidden to not map to the display property, then a lot of the code authors write would be bogus in its presence. I've seen tons of loops through the parent chain checking for display: none, and hidden not being implemented in terms of display would make those loops more complicated for authors to get right. This one (https://bugzilla.mozilla.org/show_bug.cgi?id=1381071#c1) is one that comes to mind, but I'm sure there's a gazillion examples.

I'm obviously not arguing that we should make breaking changes to hidden right now. But the bug you link is interesting - the fact that they are checking style instead of DOM, while walking the DOM tree, is causing a performance slowdown due a bug in style computation. If hidden strictly meant "not visible" (rather than the more subtle meaning it has), and it wasn't mapped to display, (leaving aside that they're also checking visibility), they could walk the DOM tree checking for the hidden attribute instead, with no need to (re)compute style. So I'm not clear on how this is an argument for exposing behaviour via computed style, since they have to walk the DOM tree in any case.

For inert, they could do the same parent chain walk, but just check the .inert IDL attribute.

CSS already controls focus behavior, via both display, overflow, and visibility, right?

Yes and no. display and visibility affect whether an element is perceivable to users, and non-perceivable elements don't take focus because that would be a very confusing and inconsistent user experience. overflow may make the container element focusable, so that users may use the arrow keys to scroll content.


I think our remaining disagreement (other than adding a new pointer-events value, discussed above) regarding having inert affect the computed style properties is mostly around:

  1. Is there value to authors in being able to use getComputedStyle() to detect some, but not all effects of inert?
    • I'm honestly still not convinced on this. It would be good to have a specific use case to demonstrate the value.
  2. If there is value, is it worth the extra effort to spec how a user agent can get access to an :inert pseudo-selector, but authors can't (or to begin a debate about whether we should ship an :inert pseudo-selector)?
  3. If we answer yes to (1) and (2), does that work need to happen before we ship inert? Is it worth delaying getting the value of inert to users by however long that work would take?
    • You pointed out that there may be a compatibility issue with getComputedStyle() returning different things depending on implementation. How would this compatibility issue cause problems?

Are there any other remaining disagreements I've missed?

@emilio
Copy link
Contributor Author

emilio commented Jun 22, 2020

If modifying the behavior of hit testing is required for inert, I'd say that it makes sense to make it expresable via the existing switch (pointer-events).

I strongly disagree.

This is equivalent to exposing part of inert's behaviour in isolation, which I've already argued against repeatedly. If people want inert-like hit testing behaviour, they should use inert.

While I'm sympathetic to that argument, having a hit-testing behavior that you can't achieve via the existing hit-testing switches seems more wrong to me than exposing this particular capability (which is likely to be useful in other ways).

(Also, pointer-events is a weird fit for HTML in the first place, in my opinion. pointer-events: none; is very counter-intuitive in what it actually means.)

That may be fair, but it's not going anywhere, so I'd rather not introduce yet another out of band way of changing hit-testing behavior.

I'm pretty sure that if we changed hidden to not map to the display property, then a lot of the code authors write would be bogus in its presence. I've seen tons of loops through the parent chain checking for display: none, and hidden not being implemented in terms of display would make those loops more complicated for authors to get right. This one (https://bugzilla.mozilla.org/show_bug.cgi?id=1381071#c1) is one that comes to mind, but I'm sure there's a gazillion examples.

I'm obviously not arguing that we should make breaking changes to hidden right now. But the bug you link is interesting - the fact that they are checking style instead of DOM, while walking the DOM tree, is causing a performance slowdown due a bug in style computation. If hidden strictly meant "not visible" (rather than the more subtle meaning it has), and it wasn't mapped to display, (leaving aside that they're also checking visibility), they could walk the DOM tree checking for the hidden attribute instead, with no need to (re)compute style. So I'm not clear on how this is an argument for exposing behaviour via computed style, since they have to walk the DOM tree in any case.

I'm not sure what you're suggesting, but they'd also need to check the style, on top of hidden, unless you're also suggesting that display: none and visibility: hidden shouldn't exist. The bug in the style engine was a typo of mine anyhow :/, but it seems totally unrelated.

For inert, they could do the same parent chain walk, but just check the .inert IDL attribute.

If you want to answer "is this node inert?" that's the same thing with or without this proposed change. However to answer "is this node selectable?" they'd have to both check user-select and inert. Making inert affect user-select seems worth it.

I think our remaining disagreement (other than adding a new pointer-events value, discussed above) regarding having inert affect the computed style properties is mostly around:

1. Is there value to authors in being able to use `getComputedStyle()` to detect some, but not all effects of `inert`?
   
   * I'm honestly still not convinced on this. It would be good to have a specific use case to demonstrate the value.

I think there is clear author value, but I'm happy to be convinced otherwise. The use case can be as simple of "checking whether node X is selectable?". inert would make it more complex if it didn't affect the existing CSS properties.

2. If there is value, is it worth the extra effort to spec how a user agent can get access to an `:inert` pseudo-selector, but authors can't (or to begin a debate about whether we should ship an `:inert` pseudo-selector)?

There's no need to specify internal pseudo-classes that aren't exposed to web content, just their effects. There's no need to implement it using my pseudo-code in the OP, you could write it with three lines of code in Blink's StyleAdjuster, fwiw :)

I think specifying that inert nodes must get a computed user-select value of none is much simpler than specifying the equivalent of user-select: none, etc. in prose in the HTML spec. You can specify it as "behaves us" too, I guess, though that makes authors' lives harder.

3. If we answer yes to (1) and (2), does that work need to happen before we ship `inert`? Is it worth delaying getting the value of `inert` to users by however long that work would take?
   
   * You pointed out that there may be a compatibility issue with `getComputedStyle()` returning different things depending on implementation. How would this compatibility issue cause problems?

As I said, I don't think it's a massive task. In fact, I think making this change would end up with a simpler specification.

I'd personally rather get it done before shipping inert, because it's a web observable, but I mean, it's your engine, you can ship whatever you want.

I don't have a super-clear example of how changing this in the future could cause problems, but I don't think that we should make breaking changes if we can avoid them before shipping, and I've seen sufficiently bizarre compat issues before to be wary of any gratuitous breaking change.

I'm happy to punt on this if folks from other browser engines don't agree with me or what not.

Are there any other remaining disagreements I've missed?

No, that was a great summary, thank you.

dbaron pushed a commit to mozilla/standards-positions that referenced this issue Jul 9, 2020
Mark it as worth-prototyping as per this comment[1] and following.

Closes #4288, let's continue the more technical discussion somewhere
else, like in whatwg/html#5650.

[1]: #174 (comment)
@asurkov asurkov mentioned this issue Aug 31, 2020
@smfr
Copy link

smfr commented Aug 4, 2021

WebKit is staring to implement inert, and this discussion raises two questions which might illuminate how an implementation works.

First, does inert alter the behavior of elementFromPoint? elementFromPoint does a hit-test, and its behavior is affected by the pointer-events property.

Second, I'm unclear on how the blocked by modal dialog algorithm works if inert is not overridable in a subtree. It would be nice to spec the inert attribute and the modal dialog behavior in a way that shares the same fundamentals, and not just "act as if every node in the document had the inert attribute applied".

@nt1m
Copy link
Member

nt1m commented Sep 20, 2021

@chrishtr Can we move forward on this? I'm trying to merge web-platform-tests/wpt#30866 if possible.

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Nov 16, 2021
Both Gecko and WebKit implement inertness based on CSS primitives like
forcing 'pointer-events: none' at used value time, instead of using
a magical hit testing retargeting for inert.

And in whatwg/html#5650, the CSSWG resolved
that this is the right approach.

Then rather than tracking inertness using Node::IsInert, it will just be
simpler to use ComputedStyle::IsInert. This patch implements that.

There are some differences, like nodes in a 'display: none' subtree
won't have a ComputedStyle so we can't really tell if they are inert,
but in practice this doesn't matter since they are not rendered.

Bug: 692360

TEST=StyleResolverTest.IsInertWithAttributeAndDialog
TEST=StyleResolverTest.IsInertWithDialogs
TEST=StyleResolverTest.IsInertWithFullscreen
TEST=StyleResolverTest.IsInertWithFrameAndFullscreen
TEST=StyleResolverTest.IsInertWithBackdrop

Change-Id: I4f8c87ea2fbc17f57290edaf185cdff4aebb7e58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3263937
Reviewed-by: Rune Lillesveen <[email protected]>
Commit-Queue: Oriol Brufau <[email protected]>
Cr-Commit-Position: refs/heads/main@{#941651}
@annevk annevk closed this as completed in 755ae90 Mar 10, 2022
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Both Gecko and WebKit implement inertness based on CSS primitives like
forcing 'pointer-events: none' at used value time, instead of using
a magical hit testing retargeting for inert.

And in whatwg/html#5650, the CSSWG resolved
that this is the right approach.

Then rather than tracking inertness using Node::IsInert, it will just be
simpler to use ComputedStyle::IsInert. This patch implements that.

There are some differences, like nodes in a 'display: none' subtree
won't have a ComputedStyle so we can't really tell if they are inert,
but in practice this doesn't matter since they are not rendered.

Bug: 692360

TEST=StyleResolverTest.IsInertWithAttributeAndDialog
TEST=StyleResolverTest.IsInertWithDialogs
TEST=StyleResolverTest.IsInertWithFullscreen
TEST=StyleResolverTest.IsInertWithFrameAndFullscreen
TEST=StyleResolverTest.IsInertWithBackdrop

Change-Id: I4f8c87ea2fbc17f57290edaf185cdff4aebb7e58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3263937
Reviewed-by: Rune Lillesveen <[email protected]>
Commit-Queue: Oriol Brufau <[email protected]>
Cr-Commit-Position: refs/heads/main@{#941651}
NOKEYCHECK=True
GitOrigin-RevId: ab14f17b660d05dc15925fea64f3cfd92c6f1ed1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants