Use old event propagation if path is not reasonable (eg. detached DOM). Fixes #4452

[jimfb]

Use old event propagation if path is not reasonable (eg. detached DOM). Fixes #4452

[jimfb]

@spicyj @sebmarkbage

[sophiebits]

Do we know if this is a Chrome bug?

[jimfb]

It's hard to know for sure. Section 3.7.4 of the spec (https://dom.spec.whatwg.org/#dispatching-events) would seem to indicate that the event path sould either be a static ordered list of all the node's ancestors in tree order, or an empty list. In this case, path is a list containing only one element (the target), which seems wrong by my reading. Also reading the event path algorithm spec (http://www.w3.org/TR/shadow-dom/#event-paths), my interpretation is that it should contain all the parent nodes. Regardless, it's the most current version of chrome and we run into it on fb home, so we need to deal with it.

[zpao]

My reading of that is also that Chrome is wrong. The node is participating in a tree (it has a parent).

Would be good to know for certain & loop in Chrome devs (with a non-React test case). This isn't too hard and is something we should do if we suspect a browser bug (especially for one with an open tracker). That way we know if we can remove the code soon (until version N+1 has reached saturation) or if we have to work around it forever.

[jimfb]

Reported to Chromium's public tracker: https://code.google.com/p/chromium/issues/detail?id=513031&thanks=513031&ts=1437605131
And also reported to Google Chrome's private tracker.

But the goal of this thread here was/is to merge this diff :P. Any objections?

[sophiebits]

I don't understand web components so I'll let @sebmarkbage approve if he comes back to his desk.

[sebmarkbage]

We should have a bailout from that infinite loop if anything unreasonable happens. Unblocking to fix master but we should have a more resilient bailout so that we don't infinite loop once more browsers starts introducing bugs (which is all of them).

[jimfb]

Added a bailout in #4502 so we don't infinite loop.

We can't (easily) move the event emission out of this loop because we need to keep re-setting the event target as we walk up. Switching to the old behavior would be wrong behavior, and we don't expect anyone to ever hit this unless another browser introduces another bug (different from the existing chrome bug). It's mostly just a sanity check at this point, and isn't worth a large refactor to implement a more graceful failure condition.