Either content elements or shadow elements should not be treated as insertion points if they are no…

Either content elements or shadow elements should not be treated as insertion points if they are not in document.

The relevant spec update: https://github.com/w3c/webcomponents/commit/69fbceabe01786ba3740728a6b86f5ec958f69b4

This behavior is similar to that of styles, which are not calculated for disconnected nodes.

This change also makes it possible to add an assertion, ASSERT(shadowRoot), for rootNode of an insertionPoint in InsertionPoint::isActive.

BUG=339029
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166647

[hayato, 2014-01-29 08:31:19]

This patch is not ready for a review.
The patch depends on several patches which are not landed yet.

[esprehn_google.com, 2014-01-29 08:44:16]

Why do you want to make this change? The distribution system works fine on
detached subtrees. I don't think we should make this change, it's worked
fine on them for well over a year now.

There is some weird bug with that bit, but disabling this feature doesn't
seem like the right fix. We should just figure out the bug.

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[hayato, 2014-01-29 08:51:34]

On 2014/01/29 08:44:16, esprehn_google.com wrote:
> Why do you want to make this change? The distribution system works fine on
> detached subtrees. I don't think we should make this change, it's worked
> fine on them for well over a year now.

I might miss some use cases, however, I think no one gets benefits from this.
If we can omit the unnecessary calculation, we should omit that.

> 
> There is some weird bug with that bit, but disabling this feature doesn't
> seem like the right fix. We should just figure out the bug.

This patch is not related at all to the
https://code.google.com/p/chromium/issues/detail?id=338987.
bug 338987 should be fixed independently.

[hayato, 2014-01-31 05:38:29]

PTAL.
Now the patch is ready for a review.

[hayato, 2014-02-06 10:11:20]

Could someone review this?

[Hajime Morrita, 2014-02-06 19:21:31]

lgtm.

[Hajime Morrita, 2014-02-06 19:21:36]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-02-06 19:21:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hayato@chromium.org/133213008/20001

[commit-bot: I haz the power, 2014-02-06 19:56:09]

Change committed as 166647

[esprehn_google.com, 2014-02-08 23:14:43]

This patch doesn't seem right, now event dispatch is all busted inside
detached trees that use shadows and distribution. That means removing a
polymer element from the document and calling methods on it is probably
broken.

We should revert this and allow distribution in detached trees, or we
should remove all the event retargeting logic (but that's crazy).

Event dispatch shouldn't take totally different paths if you're in the
document or not.

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[hayato, 2014-02-10 03:45:15]

Thank you. Could you see the comment in the spec bug?

https://www.w3.org/Bugs/Public/show_bug.cgi?id=24421#c3

I am afraid that you missed my comment. I should have mentioned that in this
review thread too.

The fact that event dispatching for disconnected nodes has been broken for years
is good enough for me to believe Polymer doesn't depend on event dispatching for
disconnected nodes. Thought I am not 100% sure. It might be better to make sure
that.

[hayato, 2014-02-10 05:20:14]

A revert of this CL has been created in
https://codereview.chromium.org/156653004/ by hayato@chromium.org.

The reason for reverting is: This patch might cause inconsistency between
Polyfill and native Shadow DOM. It might be better to revert this patch until we
have clear consensus for this change..