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

Re-add the inert="" attribute #1474

Closed
wants to merge 1 commit into from
Closed

Re-add the inert="" attribute #1474

wants to merge 1 commit into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Jun 29, 2016

This reverts the edits made in 5ddfc78 (and b0ec716) which removed the inert attribute. The inert attribute was originally added in 2fb24fc as part of the <dialog> feature. It is currently being re-considered as part of the https://github.com/WICG/inert proposal.


This pull request is not meant to be merged until we have sufficient implementer support. @alice from Chrome is interested in implementing, but we'll need more than that before we can merge. But in the meantime, this pull request can serve as a useful concretization of https://github.com/WICG/inert, in terms of the spec change that proposal intends to effect.

This reverts the edits made in 5ddfc78 (and b0ec716) which removed the inert attribute. The inert attribute was originally added in 2fb24fc as part of the <dialog> feature. It is currently being re-considered as part of the https://github.com/WICG/inert proposal.
@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest do not merge yet Pull request must not be merged per rationale in comment labels Jun 29, 2016
@annevk
Copy link
Member

annevk commented Jun 30, 2016

Paging @notwaldorf, @smaug----, @dbolter, @travisleithead, and @hober for comments.

@smaug----
Copy link

Really hard to say, since the current spec underdefines how inert works.
For example mouse event handling, is the click supposed to go to the parent node or to the element which defines the css box underneath the inert area?

@bkardell
Copy link
Contributor

@smaug---- can you clarify what you mean by "or to the element which defines the css box underneath the inert area?" I don't understand this. I would think an inert element would just preventDefault with capture essentially.

@alice
Copy link
Contributor

alice commented Jun 30, 2016

I don't think the spec does underdefine how inert works.

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

When a node is inert, then the user agent must act as if the node was absent for the purposes of targeting user interaction events, may ignore the node for the purposes of text search user interfaces (commonly known as "find in page"), and may prevent the user from selecting text in that node. User agents should allow the user to override the restrictions on search and text selection, however.

So, as Brian said, it's equivalent to preventDefault with capture - the event "never happens".

@smaug----
Copy link

preventDefault() doesn't affect at all to event "happening". Event listeners later in the capture phase or in target or bubble phase still get the event. So, don't know what preventDefault() has to do with this.
(stopPropagation() + preventDefault() would be a bit more effective ;)).
But even then, apparently the event actually is supposed to happen in the non-inert parent, but is that event retargeting supposed to happen during hittesting or later during dom event dispatch ?

@alice
Copy link
Contributor

alice commented Jun 30, 2016

Ok, I see the issue now. I think this hasn't been an issue previously because with <dialog> only the dialog block is non-inert. So to put it another way, I see three options:

  1. Is the element "transparent" for click events (click events go to the next non-inert element in the hit test stack) or
  2. does it capture the event and pass it up to its parent, or
  3. does it swallow the event altogether? (This was my assumption, but now I see that's probably unrealistic.)

Does that make sense to you as a framing?

@alice
Copy link
Contributor

alice commented Jun 30, 2016

My reading of the spec language is that (1) above makes the most sense.

@smaug----
Copy link

yeah. And from the spec my assumption was "probably (2) [parent meaning parentNode in DOM tree] but perhaps the spec author means (1) after all"

@alice
Copy link
Contributor

alice commented Jun 30, 2016

Well "act as if the node was absent" to me suggests "pretend the node isn't in the DOM at all but there's just coincidentally some stuff rendered there of mysterious origin", which would tend to support (1).

I do think as well that in practice this situation is unlikely to arise, but we do definitely need to understand how it should act if it does.

@domenic
Copy link
Member Author

domenic commented Jul 6, 2016

One issue that came up in @alice's prototyping is that it might be good to add [inert] * { cursor: default; } to the spec. See this polyfill demo, which has to add it manually: http://codepen.io/bkardell/pen/WxjgjL

@alice
Copy link
Contributor

alice commented Jul 6, 2016

Actually, it's included in the polyfill: https://github.com/alice/inert/blob/master/inert.js#L427 - Brian added it to his demo before it was added to the polyfill, but it's no longer necessary using that polyfill.

However, I agree it should be part of the spec.

@zcorpan
Copy link
Member

zcorpan commented Aug 18, 2016

Note that the selector needs to be more specific than

a:link[rel~=help i], a:visited[rel~=help i],
area:link[rel~=help i], area:visited[rel~=help i] { cursor: help; }

(or we should drop the help cursor if nobody implements it, haven't checked)

@hober
Copy link
Contributor

hober commented Jul 2, 2018

As this is still marked 'needs implementor interest': we're interested.

@robdodson
Copy link

Would it be fair to say the two remaining bits of work are:

Are there any others that folks can think of?

@domenic
Copy link
Member Author

domenic commented Jul 11, 2018

Super-great to have multi-implementer interest!

@robdodson, your list seems right. (Plus rebasing.) Would you or @alice be able to take over this pull request? Happy to give write access to non-master branches to either of you.

We'll also need web platform tests; they've become a required part of the working mode since this PR was originally posted. That will be especially good for getting interop on the event behavior (using webdriver to simulate a click).

@domenic domenic added needs tests Moving the issue forward requires someone to write tests and removed needs implementer interest Moving the issue forward requires implementers to express interest labels Jul 11, 2018
@alice
Copy link
Contributor

alice commented Aug 8, 2018

Working on updating the spec now.

Re click behaviour, I see there is now (confusingly it seems to date back to 2014?) this paragraph clarifying the behaviour:

<p class="example">For example, consider a page that consists of just a single <span>inert</span>
 paragraph positioned in the middle of a <code>body</code>. If a user moves their pointing device
 from the <code>body</code> over to the <span>inert</span> paragraph and clicks on the paragraph,
 no <code data-x="event-mouseover">mouseover</code> event would be fired, and the <code
 data-x="event-mousemove">mousemove</code> and <code data-x="event-click">click</code> events would
 be fired on the <code>body</code> element rather than the paragraph.</p>

Does that do the job? That seems to go with [3] from my comment above.

@alice
Copy link
Contributor

alice commented Aug 8, 2018

Here's my attempt at adding some advice to use the default cursor: alice@f3a8b83

@alice
Copy link
Contributor

alice commented Aug 8, 2018

I added some WPT tests, please let me know if there are some other cases you think I should cover.

@domenic
Copy link
Member Author

domenic commented Aug 9, 2018

So, both the click paragraph you cite, and the cursor note you added, are in non-normative text (an example, and a note, respectively). They don't really solve the problem, since they don't give any instructions user agents must follow.

For cursor, I see two paths:

  • Pull out your "User agents should show the default cursor" into normative text; or
  • Add a section similar to https://html.spec.whatwg.org/#hidden-elements (smaller though, I'd imagine) giving a stylesheet rule for [inert] { cursor: default; }

The main difference is that the first would also impact things made inert by dialog (e.g. if someone resized the dialog's ::backdrop to not cover them).

For event behavior, I'm less clear on what the correct normative text is, but it seemed like earlier comments were on the right track. They just need to be transcribed into normative spec text.

@alice
Copy link
Contributor

alice commented Jan 8, 2019

I updated my version of this PR:

  • I pulled the cursor out into some normative text in the "Inert subtrees" section (to capture e.g. the dialog case).
  • I re-worked the section in "Inert subtrees" dealing with event behaviour because some discussions prompted me to feel that the existing language is wrong:
    • Suppose you had an inert container div with some content in it obscuring a non-inert button (which, let's say, has the same dimensions as the div). The current language says the inert div should be considered not to be present, meaning that the button would get the click event. This seems dangerous and wrong.
    • Instead, with my update, the click event would be targeted at the div, but no event listeners would fire.

@alice
Copy link
Contributor

alice commented Jan 10, 2019

Friendly ping :) Should I make a new actual PR with my changes?

@domenic
Copy link
Member Author

domenic commented Jan 10, 2019

It would be good to rebase on master then force-push to this branch, but no need to close this PR and create a new one.

I'll try to review soon!

@domenic
Copy link
Member Author

domenic commented Jan 10, 2019

Oh, I totally forgot that this was my own PR, and not yours. Please do create your own new PR, oops!

@domenic
Copy link
Member Author

domenic commented Jan 10, 2019

I re-worked the section in "Inert subtrees" dealing with event behaviour because some discussions prompted me to feel that the existing language is wrong:

The new text doesn't seem right though. It has the following normative consequence:

document.body.addEventListener("foo", () => console.log("foo event fired"));
document.body.inert = true;

document.body.dispatchEvent(new Event("foo")); // doesn't log anything

In addition it's a pretty heavy monkeypatch to the event dispatch algorithm, although I guess that was already true for the previous patch in some ways.

It seems like you're going for (3) in #1474 (comment), right? My stab at some text for that would be "User interactions which would otherwise cause events to be targeted at an inert element must instead do nothing". Not sure that's great either...

Here are two ways to help formalize things a bit more:

@alice alice mentioned this pull request Jan 14, 2019
@alice
Copy link
Contributor

alice commented Jan 14, 2019

New PR: #4288

@domenic
Copy link
Member Author

domenic commented Jan 14, 2019

Moving to #4288!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements do not merge yet Pull request must not be merged per rationale in comment needs tests Moving the issue forward requires someone to write tests
Development

Successfully merging this pull request may close these issues.

8 participants