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

[Shadow]: Update constraints around stopping events (bugzilla: 20247) #61

Closed
hayatoito opened this issue May 25, 2015 · 17 comments
Closed

Comments

@hayatoito
Copy link
Contributor

Title: [Shadow]: Update constraints around stopping events (bugzilla: 20247)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247


comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c0
Dimitri Glazkov wrote on 2012-12-05 15:27:41 +0000.

"The following events must always be stopped at the nearest shadow boundary:"

Since we relaxed the lower boundary in regard to events, we should similarly not stop events there.


comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c1
Dimitri Glazkov wrote on 2012-12-05 15:31:06 +0000.

Also, this needs to account for the nodes, distributed to the insertion point. Events from those should be heard outside of the shadow tree.


comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c2
Dimitri Glazkov wrote on 2012-12-05 17:54:57 +0000.

And we need to write down the rationale behind stopping these events.


comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c3
Dimitri Glazkov wrote on 2012-12-06 23:08:37 +0000.

I am trying to come up with a general rule of thumb. I guess is something like:

If the event is retargeted, does event's meaning change?

For example, if a "load" event is heard at a shadow host, it conveys the meaning of shadow host somehow completing loading, which is not true, because all it means that some element inside of host's shadow tree has completed loading.

Conversely, if an "click" event is heard a a shadow host, it says that the shadow host was clicked. Since shadow tree is how shadow host is rendered, the meaning of the event stays the same.

WDYT?


comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c4
Anne wrote on 2012-12-07 10:50:53 +0000.

But if the shadow tree actually did load something via XMLHttpRequest and wanted to convey that you're lost.

I suppose the current set of events is based on what WebKit needed? It seems unclear that will match the needs of developers when this gets out in the wild.


comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c5
Dimitri Glazkov wrote on 2012-12-07 16:42:25 +0000.

(In reply to comment #4)

But if the shadow tree actually did load something via XMLHttpRequest and
wanted to convey that you're lost.

Then just fire a "load" event on the shadow host?

I suppose the current set of events is based on what WebKit needed? It seems
unclear that will match the needs of developers when this gets out in the
wild.

We only wanted to stop a "selectstart" in WebKit. The rest of the list is based on Microsoft's experience with HTC:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=15804


comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c6
Dimitri Glazkov wrote on 2012-12-07 23:58:17 +0000.

From a thread on whatwg: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-December/038283.html

It sounds though like you'd want a different approach to this. What if
I have a

Then you probably don't want the "load" events of

It's an interesting question, though. Along with "load", such implementation detail may dispatch a whole bunch of other events (http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#mediaevents).

Most of these events--at least, following my reasoning--seem like they should just be kept in the shadow tree.

I wonder if we would be better off reversing the condition and stopping ALL events, except a set of events whose meaning stays clear after retargeting (like "click").


comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c7
Dimitri Glazkov wrote on 2012-12-07 23:59:22 +0000.

*** Bug 20248 has been marked as a duplicate of this bug. ***


comment: 8
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c8
Dimitri Glazkov wrote on 2013-04-01 23:21:07 +0000.

Also interesting comments in bug 20633.


comment: 9
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c9
Dimitri Glazkov wrote on 2013-04-01 23:21:12 +0000.

*** Bug 20633 has been marked as a duplicate of this bug. ***


comment: 10
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c10
Anne wrote on 2013-04-02 08:29:11 +0000.

http://lists.w3.org/Archives/Public/public-webapps/2013JanMar/thread.html#msg312 is how we should solve this imo.


comment: 11
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c11
Dimitri Glazkov wrote on 2013-05-08 17:44:43 +0000.

Bug 21269 has a great discussion around use cases to arbitrarily control event stopping.


comment: 12
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c12
Dimitri Glazkov wrote on 2013-05-08 17:45:13 +0000.

*** Bug 21269 has been marked as a duplicate of this bug. ***


comment: 13
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c13
Dimitri Glazkov wrote on 2013-07-17 22:02:55 +0000.

*** Bug 21400 has been marked as a duplicate of this bug. ***


comment: 14
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c14
Steve Orvell wrote on 2013-09-26 00:25:33 +0000.

A polymer user had a question about this today which prompted some discussion (www.polymer-project.org).

I think at the least the spec needs to be updated to explain the logic behind stopping events. Most of the events that are stopped are non-bubbling events (except 'change'). Since these don't bubble, why must they be stopped? Is this to prevent capturing?

Stopping events poses some problems:

  1. Are the events stopped entirely by name? For example, if a synthetic event is dispatched called 'load' will it be stopped? If so, that seems bad.
  2. In the context of custom elements, the developer may want control over whether or not the event stops at the shadowDOM boundary.

In light of this complexity, it seems like a more complex api would be needed to be able to hide shadow events effectively.

Instead of introducing this complexity, perhaps we can remove the concept of stopped events entirely and instead rely on bubbling behavior. Non-bubbling events will naturally not propagate outside shadowRoot. Events that are captured would just be retargeted.


comment: 15
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c15
Hayato Ito wrote on 2015-02-13 08:28:28 +0000.

I've added a like to this issue to the spec:

aa322cb


comment: 16
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c16
Hayato Ito wrote on 2015-02-13 08:29:01 +0000.

(In reply to Hayato Ito from comment #15)

I've added a like to this issue to the spec:

https://github.com/w3c/webcomponents/commit/
aa322cb

s/like/link/.


comment: 17
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20247#c17
Anne wrote on 2015-04-23 18:43:53 +0000.

The solution here is to add a bit flag to the event dispatch algorithm whether or not the event needs to be scoped to the shadow tree. And then set it in specifications from all the relevant features, such as e.g. 's load & error events.

@hayatoito
Copy link
Contributor Author

I'd like to move this issue forward with an Anne's idea.
Let me try to spec it as a monkey patch in the Shadow DOM spec for a review.

@hayatoito
Copy link
Contributor Author

@annevk
If possible, could you check whether adae25b is a right direction or not? I appreciate any feedback.

@annevk
Copy link
Collaborator

annevk commented Jun 9, 2015

Looks good overall, added a comment about the note though: adae25b#commitcomment-11586721

hayatoito added a commit that referenced this issue Jun 11, 2015
…stor trees only if they are dispatched by the user agent.
@hayatoito
Copy link
Contributor Author

Let me reopen this issue. Looks we need more investigation.
See
adae25b#commitcomment-11603540

@hayatoito hayatoito reopened this Jun 11, 2015
@annevk
Copy link
Collaborator

annevk commented Jun 11, 2015

What we need to do is identify which event dispatching code needs to set this flag. E.g. the code dispatching events for <img> will need to set this flag. But the code dispatching events for XMLHttpRequest does not. Both dispatch error events so that's why it's important to figure out the dispatch sources.

@kojiishi
Copy link

kojiishi commented Jul 2, 2015

@annevk is it sufficient to set only when the target is participating in a tree, the same criteria to set the event path, as defined in item 4 of Dispatching events?

@annevk
Copy link
Collaborator

annevk commented Jul 2, 2015

I think we want an actual algorithm for establishing the event path (or maybe if all the tree terminology is accurate it can fall out from that?). What DOM has is somewhat nice, but a bit vague and has already led to problems with people not understanding Window being part of the path. So finding a more algorithmic way of describing it might actually be better.

@kojiishi
Copy link

kojiishi commented Jul 3, 2015

Can I confirm, if you're talking about when to set this flag, or the definition is ok but how to organize specs and upstream Events section in Shadow DOM to DOM spec?

IIUC hayato is considering to generalize the Event Paths since it's quite possible that the algorithm is shared by event path and cascading, but the cascading part is currently blocked by other higher priority discussions. It might make our lives easier to plan the upstream after it was settled down.

Apart from whether to monkey-patch in Shadow DOM spec or upstream to DOM spec, does the item 4 of 3.7. Dispatching events look the correct place to set this flag?

@hayatoito
Copy link
Contributor Author

Regarding with upstreaming,

I think we all agree that we should upstream Shadow DOM spec to DOM spec (and other specs) someday. At the same time, I think we all agree that having monkey patches in Shadow DOM spec is okay until the situation becomes stable enough.

As of now, there is no plan how to upstream. I don't think this will happen soon.

Until the time, we have to do a difficult judge, "Which spec should we update?", case by case. Yeah, that's painful.. :(

@annevk
Copy link
Collaborator

annevk commented Jul 3, 2015

@kojiishi this flag is a new argument to the dispatch algorithm. It is then used in step 4, assuming we don't need to restructure more of the dispatch algorithm to make all this work better. As a rough guide, if the code for dispatching in Blink is substantially different, there's a bug in either Blink or the specification, and we should figure out together what needs changing.

I agree that for now we should monkey patch DOM. I do hope that once our discussions settle down and a v1 is agreed upon we can work together on integrating in DOM directly. I think that will help those writing tests and implementations and might also help our overall understanding of the model.

@hayatoito
Copy link
Contributor Author

This issue looks slightly obsolete. Let me close this tentatively.

@annevk
Copy link
Collaborator

annevk commented Feb 17, 2016

Given that Shadow DOM doesn't describe how it modifies DOM's dispatch algorithm with these flags, I'm not sure we should be closing this.

@annevk annevk reopened this Feb 17, 2016
@hayatoito
Copy link
Contributor Author

Regarding the flags, the event path calculation algorithm (and accompanying algorithms) uses the flags, scoped and relatedTargetScoped (tentative name), to control the event path.

http://w3c.github.io/webcomponents/spec/shadow/#event-paths-1

@kojiishii, I might lose the context in this issue. Do you have any suggestions?

@annevk
Copy link
Collaborator

annevk commented Feb 17, 2016

So I think in part a problem I need to solve is that DOM needs to define event path calculation better. Apart from the node tree Indexed DB also needs a hook for this. I opened w3c/IndexedDB#66 to discuss their requirements.

It would be useful to know how you think event path calculation works at a low-level.

My intuition was that dispatch asks event target for an event path and that event target has an algorithm that returns the event target itself (the default event path for most objects), but can be overwritten by special event targets, such as nodes and certain Indexed DB objects to return a list of event targets.

@rniwa
Copy link
Collaborator

rniwa commented Feb 17, 2016

Yeah, that seems a clear design (asking the event target for the path).

@annevk
Copy link
Collaborator

annevk commented Feb 18, 2016

I now clarified in the DOM Standard how this is done (and removed a monkey patch from HTML in the process).

@hayatoito
Copy link
Contributor Author

As you see, the current Shadow DOM spec's event path calculation should be applied to an event target only if the event target is a Node in a node tree. If the event target is not a Node, this should not be applied. It looks the Shadow DOM spec editors, including me, have never considered such a case.

Let me clarify it as a tentative fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants