Bug 16176 - [Shadow]: What should we do if an event happens on light child which is distributed to a insertion point.
[Shadow]: What should we do if an event happens on light child which is distr...
Status: RESOLVED FIXED
Product: WebAppsWG
Classification: Unclassified
Component: Component Model
unspecified
PC All
: P2 normal
: ---
Assigned To: Dimitri Glazkov
public-webapps-bugzilla
:
Depends on:
Blocks: 17050
  Show dependency treegraph
 
Reported: 2012-03-01 12:53 UTC by Hayato Ito
Modified: 2012-05-16 02:28 UTC (History)
4 users (show)

See Also:


Attachments
Example DOM tree and its composed shadow tree. (179.99 KB, image/jpeg)
2012-04-06 10:14 UTC, Hayato Ito
Details
Sample Shadow DOM Tree (59.77 KB, image/svg+xml)
2012-05-10 20:58 UTC, Dimitri Glazkov
Details
Sample DOM Tree (96.71 KB, text/html)
2012-05-10 21:03 UTC, Dimitri Glazkov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2012-03-01 12:53:59 UTC
Let me explain using the following example.

Case A)

DOM Tree is here:
  <div id='a'>
    <div id='b'>
    </div>
  </div>

An element #a has a ShadowRoot, which has a <content> element as a child.
  <shadow-root>
    <content>

A result of node distribution algorithm is here (it's the same to original DOM tree except for there is a shadow boundary between #a and #b):
  <div id='a'>
    -- <shadow boundary> --
      <div id='b'>
      </div>
  </div>



Case B)

DOM Tree is here:
  <div id='a'>
    <div id='b'>
    </div>
  </div>

There is no ShadowRoot.



Suppose a user move mouse cursor from '#b' to '#a'.

In Case A)
- 'mouseout' event should happen only at '#b' and stop here since there is a shadow boundary between '#a' and '#b'. Mouse cursor moves within a shadowHost '#a', so a shadowHost '#a' should not receive any mouseout events.

In Case B)
- 'mouseout' event should bubble up usually. An 'mouseout' event (currentTarget='#a', target='#b') happens also at '#a' as well as at '#b'.


I am afraid that users might be upset in case A) because it behaves differently than normal DOM tree.

I've found this issue when I was implementing new event dispatching logic in WebKit and apply a new event dispatching logic to <details> and <summary>:

 <details>
   <summary>summary<summary>
   details
  </details>


If mouse moves form <summary> to <details>, 'mouseout' event should happen at <summary> and <details> (at BUBBLE).
But WebKit uses ShadowRoot internally, 'mouseout' event does not happen at <details> if I apply the new event dispatching algorithm.

I'd like to treat these cases uniformly.

Which should I fix? I think this is worth discussing because we might want to modify the spec.
Is there any ideas?
Comment 1 Dimitri Glazkov 2012-03-01 17:27:21 UTC
You're right! I think we need to fix this somehow. Let me think on this...
Comment 2 Dimitri Glazkov 2012-03-08 23:40:56 UTC
I think we should count lower boundary as -1 in our calculations. Will this work?

In Case A, boundary count: 1 + (- 1) = 0.
Comment 3 Dimitri Glazkov 2012-03-13 20:36:03 UTC
Here's what I've come up with: http://dvcs.w3.org/hg/webcomponents/rev/378dd45d7b80

Basically, whenever calculating ancestors for relatedTarget, we must ignore the upper boundary. This way, the node will just appear as a child of a shadow host, not as a something that's distributed into an insertion point.

WDYT?
Comment 4 Dimitri Glazkov 2012-03-13 20:38:06 UTC
Closing to check off my list, please reopen if I messed up.
Comment 5 Hayato Ito 2012-03-14 05:40:39 UTC
Oh. How clever is that you modified only calculation of ancestors for relatedTarget!
That sounds a nice idea for me. Let me think further and modify the current EventDispatching algorithm.
If I find a issue, let me reopen this.


(In reply to comment #3)
> Here's what I've come up with:
> http://dvcs.w3.org/hg/webcomponents/rev/378dd45d7b80
> 
> Basically, whenever calculating ancestors for relatedTarget, we must ignore the
> upper boundary. This way, the node will just appear as a child of a shadow
> host, not as a something that's distributed into an insertion point.
> 
> WDYT?
Comment 6 Hayato Ito 2012-04-06 05:02:36 UTC
Let me reopen the bug.

Suppose the following DOM tree.

<div id='top'>
  <div id='shadow-host'>
      -- <shadow-root> --
               <content />
     <div id='distributed-light-child' />
  </div>
</div>

And move mouse from '#shadow-host' to '#distributed-light-child' and think the 'mouseover' event on #distributed-light-child.

In this case:
  TARGET: #distributed-light-child
  ancestors of TARGET - [#distributed-light-child, <content>, <shadow-root>, #shadow-host, #top]

  RELATED: #shadow-host
  ancestors of RELATED - [#shadow-host, #top]

The re-targeting algorithm will go as follows:

1. Let COMMON be undefined
2. Let ANCESTOR be the lowest common ancestor:
  (ANCESTOR <- #shadow-host (lowest common ancestor.))
3. If ANCESTOR exists:
  If ANCESTOR is RELATED and is a shadow host:
   Set LIMIT to ANCESTOR     (LIMIT <- #shadow-host)
   Set ADJUSTED to RELATED   (ADJUSTED <- #shadow-host)
   Stop.3. 

So the output of retargeting algorithm would be (LIMIT=#shadow-host, ADJUSTED=#shadow-host).
That means #top does not receive 'mouseover' event, which is different from the result of the following case where a #shadow-host does does not contain a shadowRoot, so it is not 'ShadowHost' actually:

<div id='top'>
   <div id='shadow-host'>
      <div id='distributed-light-child' />
   </div>
</div>


Is this an intentional behavior?
I think that one of our goals is that we won't behavior where an events happens on light child and shadow-host from the outside of shadow host, right?


(In reply to comment #5)
> Oh. How clever is that you modified only calculation of ancestors for
> relatedTarget!
> That sounds a nice idea for me. Let me think further and modify the current
> EventDispatching algorithm.
> If I find a issue, let me reopen this.
> 
> 
> (In reply to comment #3)
> > Here's what I've come up with:
> > http://dvcs.w3.org/hg/webcomponents/rev/378dd45d7b80
> > 
> > Basically, whenever calculating ancestors for relatedTarget, we must ignore the
> > upper boundary. This way, the node will just appear as a child of a shadow
> > host, not as a something that's distributed into an insertion point.
> > 
> > WDYT?
Comment 7 Hayato Ito 2012-04-06 10:14:09 UTC
Created attachment 1108 [details]
Example DOM tree and its composed shadow tree.
Comment 8 Hayato Ito 2012-04-06 10:21:38 UTC
I've spent some time on how event dispatching should behave if light children are distributed.
Take a look at the attached image for example DOM tree and its composed shadow tree.
https://www.w3.org/Bugs/Public/attachment.cgi?id=1108

Suppose a mouse moves from '#F' (relatedTarget) to '#D' (target). In this case, on which nodes a 'mouseover' event should happen?

My initial thought in this case is:

- #D (target = #D, relatedTarget = #L)
- #C (target = #D, relatedTarget = #L)
- #J (target = #J, relatedTarget = #L)  (This might be controversial, but I think an event should be fired on #J also)
-  ... (#G should be skipped...)
- #B (target = #D, relatedTarget = #F)
- #A (target = #D, relatedTarget = #F)

This is one of ideas. If we try to keep compatibility as much as possible. That should be:

- #D (target = #D, relatedTarget = #F (not #L))
- #C (target = #D, relatedTarget = #F (not #L))
- #J (target = #J, relatedTarget = #L)
- ... (#G should be skipped...)
- #B (target = #D, relatedTarget = #F)
- #A (target = #D, relatedTarget = #F)

I am afraid that this will make the situation (and the spec) complex, but I'd like to discuss what is expected and reasonable behavior.
Comment 9 Hayato Ito 2012-04-09 08:07:12 UTC
Let me continue thinking and propose a new idea.
I prefer the latter to the former since that won't break compatibility. Let me quote that again here.

> - #D (target = #D, relatedTarget = #F)
> - #C (target = #D, relatedTarget = #F)
> - #J (target = #J, relatedTarget = #L)
> - ... (#G should be skipped...)
> - #B (target = #D, relatedTarget = #F)
> - #A (target = #D, relatedTarget = #F)


To achieve that,  section 6 should be modified as follows:

- Remove IGNORE_UPPER_BOUNDARY flag from the Section 6.
  That means we use the same algorithm to calculate ancestors both for a target and a relatedTarget.

- Define a scope of a node as follows:
  A nearest ancestor youngestShadowRoot (or Document). Ancestors should be calculated by algorithm described in Section6, but not using <content> element as insertionPoint. Use only <shadow> element as insertion points.

- Modify 6.5:
  - If the scope of currentTarget are same to the scope of TARGET (the original target DOM node of the event):
    An event should be dispatched on the currentTarget node even if the currentTarget is an ancestor of LIMIT.
    And, use TARGET as target, instead of using adjusted target.
  - If the scope of currentTarget are same to the scope of RELATED (the original related target DOM node of the event), use RELATED as relatedTarget, instead of using adjusted relatedTarget.

This rule can achieve the result:
 - At #D, its scope is same to the scope of TARGET(#D) and the scope of RELATED(#F). Use (target='#D', relatedTarget='#F')
 - At #C, its scope is same to the scope of TARGET(#D) and the scope of RELATED(#F). Use (target='#D', relatedTarget='#F')
 - At #J, its scope is different from the scope of #D and the scope of #F. Therefore use adjusted results (target = #J, relatedTarget = #L)
 - At #G, an event should not be dispatched since it's ancestor of LIMIT.
 - At #B, its scope is same to the scope of #D and the scope of #F, so an event should be dispatched on #B even if it is an ancestor of LIMIT. Use (target='#D', relatedTarget='#F')
 - At #A, its scope is same to the scope of #D and the scope of #F, so an event should be dispatched on #A even if it is an ancestor of LIMIT. Use (target='#D', relatedTarget='#F')

That preserves compatibility. At #D, #C, #B, and #A, nothing has changed before/after shadow DOM spec.
That only explains a basic idea. Is there any flaws?
Comment 10 Dimitri Glazkov 2012-05-10 00:08:17 UTC
I worked on this a bit today, using excellent examples in https://docs.google.com/a/google.com/document/d/1PY7KkTRI-doU9Hew3Mud9JBgjj5xtVqH8sY0Ui305nw/edit. Here are some findings:

1) The calculations in the referenced doc do not take insertion points or shadow roots into the account. Nothing in spec says that we shouldn't fire event on insertion point or shadow roots. So, for D, the chain of ancestors will actually be:

A - B - [ ] - G - [ ] - J - <K> - <H> - C - D

where [ ] is a shadow root and <X> is an insertion point.

2) The calculation of adjusted target is wrong. We should treat insertion points differently from the shadow roots. In fact, the whole notion of boundaries must be discarded and replaced instead with a stack. We always read the top of the stack as the relative target. When we begin, we push target node into the stack. When we encounter an insertion point as a parent, we push current target into the stack. When we pass a shadow root, we pop the stack. If there's nothing in stack, we immediately push shadow host into the stack.

This gives us a correct representation of the relative target. For example, for D, it will be:

D - D - H - H - K - K -K - H - D - D

3) relativeTarget is a variable, as Hayato-san suspects. The general idea is that we build a list of relative targets for each tree scope using adjusted target algorithm, then supply the relatedTarget of the same scope as the adjusted target. This is an easy map, constant time thing.

I will try to write this down as spec tomorrow.
Comment 11 Dimitri Glazkov 2012-05-10 00:10:58 UTC
(In reply to comment #10)

> 3) relativeTarget is a variable, as Hayato-san suspects. The general idea is
> that we build a list of relative targets for each tree scope using adjusted
> target algorithm, then supply the relatedTarget of the same scope as the
> adjusted target. This is an easy map, constant time thing.

Correction: The general idea is that we build a list of adjusted relatedTargets for each scope...
Comment 12 Hayato Ito 2012-05-10 05:57:06 UTC
I've read carefully your suggestion and now I think I understand your suggestion.

The doc was out-of-date. So I've updated the following section in the doc to reflect your suggestions: 

https://docs.google.com/a/google.com/document/d/1PY7KkTRI-doU9Hew3Mud9JBgjj5xtVqH8sY0Ui305nw/edit:

  - Example tree (I've added labels to ShadowRoots and InsertionPoints to the picture)
  - Examples (I've added ShadowRoots and InsertionPoints in the tables)

Please take a look at examples and let me tell whether this reflects your intention or not.

# I think we should paste the picture and tables from the doc here using acsii-art or something for the record. But forgive my laziness...

(In reply to comment #11)
> (In reply to comment #10)
> 
> > 3) relativeTarget is a variable, as Hayato-san suspects. The general idea is
> > that we build a list of relative targets for each tree scope using adjusted
> > target algorithm, then supply the relatedTarget of the same scope as the
> > adjusted target. This is an easy map, constant time thing.
> 
> Correction: The general idea is that we build a list of adjusted relatedTargets
> for each scope...

(In reply to comment #10)
> I worked on this a bit today, using excellent examples in
> https://docs.google.com/a/google.com/document/d/1PY7KkTRI-doU9Hew3Mud9JBgjj5xtVqH8sY0Ui305nw/edit.
> Here are some findings:
> 
> 1) The calculations in the referenced doc do not take insertion points or
> shadow roots into the account. Nothing in spec says that we shouldn't fire
> event on insertion point or shadow roots. So, for D, the chain of ancestors
> will actually be:
> 
> A - B - [ ] - G - [ ] - J - <K> - <H> - C - D
> 
> where [ ] is a shadow root and <X> is an insertion point.
> 
> 2) The calculation of adjusted target is wrong. We should treat insertion
> points differently from the shadow roots. In fact, the whole notion of
> boundaries must be discarded and replaced instead with a stack. We always read
> the top of the stack as the relative target. When we begin, we push target node
> into the stack. When we encounter an insertion point as a parent, we push
> current target into the stack. When we pass a shadow root, we pop the stack. If
> there's nothing in stack, we immediately push shadow host into the stack.
> 
> This gives us a correct representation of the relative target. For example, for
> D, it will be:
> 
> D - D - H - H - K - K -K - H - D - D
> 
> 3) relativeTarget is a variable, as Hayato-san suspects. The general idea is
> that we build a list of relative targets for each tree scope using adjusted
> target algorithm, then supply the relatedTarget of the same scope as the
> adjusted target. This is an easy map, constant time thing.
> 
> I will try to write this down as spec tomorrow.
Comment 13 Dimitri Glazkov 2012-05-10 20:58:54 UTC
Created attachment 1130 [details]
Sample Shadow DOM Tree
Comment 14 Dimitri Glazkov 2012-05-10 21:03:47 UTC
Created attachment 1131 [details]
Sample DOM Tree
Comment 15 Dimitri Glazkov 2012-05-10 21:09:21 UTC
(In reply to comment #12)
> I've read carefully your suggestion and now I think I understand your
> suggestion.
> 
> The doc was out-of-date. So I've updated the following section in the doc to
> reflect your suggestions: 
> 
> https://docs.google.com/a/google.com/document/d/1PY7KkTRI-doU9Hew3Mud9JBgjj5xtVqH8sY0Ui305nw/edit:
> 
>   - Example tree (I've added labels to ShadowRoots and InsertionPoints to the
> picture)
>   - Examples (I've added ShadowRoots and InsertionPoints in the tables)
> 
> Please take a look at examples and let me tell whether this reflects your
> intention or not.
> 
> # I think we should paste the picture and tables from the doc here using
> acsii-art or something for the record. But forgive my laziness...

These are great! I added them as attachment.
Comment 16 Dimitri Glazkov 2012-05-10 22:17:07 UTC
http://dvcs.w3.org/hg/webcomponents/rev/560fb0fda0e3
Comment 17 Dimitri Glazkov 2012-05-10 22:44:12 UTC
http://dvcs.w3.org/hg/webcomponents/rev/43aa630c7cec
Comment 18 Dimitri Glazkov 2012-05-11 22:48:26 UTC
http://dvcs.w3.org/hg/webcomponents/rev/80a9f05d1661 <-- stick a fork in it!

Folks, please look over and reopen bug if I'd done bad.
Comment 19 Hayato Ito 2012-05-14 04:10:51 UTC
Thank you for updating the spec, Dimitri!

(In reply to comment #18)
> http://dvcs.w3.org/hg/webcomponents/rev/80a9f05d1661 <-- stick a fork in it!
> 
> Folks, please look over and reopen bug if I'd done bad.

I've taken a look it. Let me reopen this issue since there are some points to be fixed.

About 6.2 Retargeting relatedTarget;

> 2. Let ANCESTOR be ADJUSTED

That should be '2. Let ANCESTOR be RELATED'.

> 1. If there is a DOM node that is a member of CANDIDATES and is in the same subtree as TARGET, let ADJUSTED be this DOM node

There may be multiple candidates in the same subree. In such case, we should pick up "the lowest'"candidate from the candidates.

> 2. Otherwise, let TARGET be the result of running the parent calculation algorithm with TARGET as input

That should not be 'the parent calculation algorithm'. The next TARGET should be either  'TARGET.parentNode() or TARGET.host(). If we use 'parent calculation algorithm' here, ADJUSTED might be a node which is not accessible from the NODE.
Comment 20 Dimitri Glazkov 2012-05-14 21:10:09 UTC
(In reply to comment #19)
> Thank you for updating the spec, Dimitri!

Thank you for reviewing it! :)

> 
> About 6.2 Retargeting relatedTarget;
> 
> > 2. Let ANCESTOR be ADJUSTED
> 
> That should be '2. Let ANCESTOR be RELATED'.

Fixed.

> 
> > 1. If there is a DOM node that is a member of CANDIDATES and is in the same subtree as TARGET, let ADJUSTED be this DOM node
> 
> There may be multiple candidates in the same subree. In such case, we should
> pick up "the lowest'"candidate from the candidates.

Ah. good point. I think I need to change the CANDIDATES collection algorithm to only pick up unique items.

> 
> > 2. Otherwise, let TARGET be the result of running the parent calculation algorithm with TARGET as input
> 
> That should not be 'the parent calculation algorithm'. The next TARGET should
> be either  'TARGET.parentNode() or TARGET.host(). If we use 'parent calculation
> algorithm' here, ADJUSTED might be a node which is not accessible from the
> NODE.

Ooh, you're right! Thanks, fixed.

http://dvcs.w3.org/hg/webcomponents/rev/6486c4deeb63
Comment 21 Dimitri Glazkov 2012-05-14 21:10:31 UTC
Please reopen if you find more bugs! :)
Comment 22 Hayato Ito 2012-05-15 04:03:56 UTC
Thank you for updating the spec. Let me reopen again :)

(In reply to comment #20)
> > 
> > > 1. If there is a DOM node that is a member of CANDIDATES and is in the same subtree as TARGET, let ADJUSTED be this DOM node
> > 
> > There may be multiple candidates in the same subree. In such case, we should
> > pick up "the lowest'"candidate from the candidates.
> 
> Ah. good point. I think I need to change the CANDIDATES collection algorithm to
> only pick up unique items.

Unfortunately, we may still have multiple candidate nodes in the same tree as a result.
That's because when we climbing up ancestors of relatedTarget, we may *re-visit* the same subtree which we already visit before.
We can see the example in the example tree. https://www.w3.org/Bugs/Public/attachment.cgi?id=1131
If an original relatedTarget is F, both B and F will be added to CANDIDATES.


> If ANCESTOR is not in the same subtree as LAST and ANCESTOR is not an insertion point:

Why do we exclude InserionPoints from candidates? InsertionPoints could be a relatedTarget, couldn't we?
Comment 23 Dimitri Glazkov 2012-05-15 18:02:10 UTC
(In reply to comment #22)
> Thank you for updating the spec. Let me reopen again :)
> 
> (In reply to comment #20)
> > > 
> > > > 1. If there is a DOM node that is a member of CANDIDATES and is in the same subtree as TARGET, let ADJUSTED be this DOM node
> > > 
> > > There may be multiple candidates in the same subree. In such case, we should
> > > pick up "the lowest'"candidate from the candidates.
> > 
> > Ah. good point. I think I need to change the CANDIDATES collection algorithm to
> > only pick up unique items.
> 
> Unfortunately, we may still have multiple candidate nodes in the same tree as a
> result.
> That's because when we climbing up ancestors of relatedTarget, we may
> *re-visit* the same subtree which we already visit before.
> We can see the example in the example tree.
> https://www.w3.org/Bugs/Public/attachment.cgi?id=1131
> If an original relatedTarget is F, both B and F will be added to CANDIDATES.

Yes, we only need to keep the *lowest* node in the CANDIDATES. Because we are traversing from the bottom, we need to only check whether a node from the subtree already exists in CANDIDATES.

> 
> 
> > If ANCESTOR is not in the same subtree as LAST and ANCESTOR is not an insertion point:
> 
> Why do we exclude InserionPoints from candidates? InsertionPoints could be a
> relatedTarget, couldn't we?

You're right. My bad.

http://dvcs.w3.org/hg/webcomponents/rev/9f40fd372168 <-- hopefully sticks this time :)
Comment 24 Dimitri Glazkov 2012-05-15 22:26:17 UTC
For posterity, here's the worksheet I used:

D: D C <H> <K> <N> [J] J [G] G [B] B A
F: F E <I> <M> <O> [L] L [G] G [B] B A

L: L [G] G [B] B A

candidates for L: --> L G B

currentTarget: D C <H> <K> <N> [J]  J  [G]  G  [B]  B A 
       target: D D <H> <K> <N> <N> <K> <K> <H> <H>  D D  
relatedTarget: B B  G   L   L   L   L   L   G   G   B B


candidates for F: -> F <I> <M> <O>
                     E  G   L  [L]
                     B [B] [G]
                     A

currentTarget: D C <H> <K> <N> [J]  J  [G]  G  [B]  B A 
       target: D D <H> <K> <N> <N> <K> <K> <H> <H>  D D  
relatedTarget: F F <I> <M> <M> <M> <M> <M> <I> <I>  F F
Comment 25 Hayato Ito 2012-05-16 02:28:19 UTC
Thank you for updating the spec. Looks good to me!
We won finally.

(In reply to comment #23)
> (In reply to comment #22)
> > Thank you for updating the spec. Let me reopen again :)
> > 
> > (In reply to comment #20)
> > > > 
> > > > > 1. If there is a DOM node that is a member of CANDIDATES and is in the same subtree as TARGET, let ADJUSTED be this DOM node
> > > > 
> > > > There may be multiple candidates in the same subree. In such case, we should
> > > > pick up "the lowest'"candidate from the candidates.
> > > 
> > > Ah. good point. I think I need to change the CANDIDATES collection algorithm to
> > > only pick up unique items.
> > 
> > Unfortunately, we may still have multiple candidate nodes in the same tree as a
> > result.
> > That's because when we climbing up ancestors of relatedTarget, we may
> > *re-visit* the same subtree which we already visit before.
> > We can see the example in the example tree.
> > https://www.w3.org/Bugs/Public/attachment.cgi?id=1131
> > If an original relatedTarget is F, both B and F will be added to CANDIDATES.
> 
> Yes, we only need to keep the *lowest* node in the CANDIDATES. Because we are
> traversing from the bottom, we need to only check whether a node from the
> subtree already exists in CANDIDATES.
> 
> > 
> > 
> > > If ANCESTOR is not in the same subtree as LAST and ANCESTOR is not an insertion point:
> > 
> > Why do we exclude InserionPoints from candidates? InsertionPoints could be a
> > relatedTarget, couldn't we?
> 
> You're right. My bad.
> 
> http://dvcs.w3.org/hg/webcomponents/rev/9f40fd372168 <-- hopefully sticks this
> time :)