WebReflection / document-register-element

A stand-alone working lightweight version of the W3C Custom Elements specification
ISC License
1.13k stars 116 forks source link

Subchilds does not trigger detachedCallback in IE11 when removed through innerHTML #101

Closed johan-ludvigsson-stratsys closed 6 years ago

johan-ludvigsson-stratsys commented 7 years ago

Found out today that there is a bug in IE11 MutationObserver where childnodes to directly removed nodes are missing: https://connect.microsoft.com/IE/feedback/details/817132/ie-11-childnodes-are-missing-from-mutationobserver-mutations-removednodes-after-setting-innerhtml

I've put together a quick jsfiddle with an examle using the latest CDN-version of your polyfill: https://jsfiddle.net/qfmLvdyw/4/

Chrome, Firefox and Edge handles the case correctly as expected. Using .removeChild() instead of .innerHTML() seems to be working correctly in all browser. I couldn't find any information on the known caveats, so I'm not sure which approach you consider most valid. Should we look into a complete polyfill for MutationObserver or do you think it is something reasonable that the registerElement polyfill should handle?

WebReflection commented 7 years ago

I don't think I want to overload this repo with an overkilling MutationObserver polyfill only because IE11 is broken, it sounds more trouble than solution.

I rather force IE11 to use the fallback based on Mutation events like it is for older browsers, including IE9 and 8.

I'll try to use your test case and see what I can do to degrade IE11 and make it reliable.

Thanks so far.

johan-ludvigsson-stratsys commented 7 years ago

Since the issues is regarding innerHTML, is it something that could be complemented to the innerHTML helper function instead? As I understand it only dealt with the createdCallback?

WebReflection commented 7 years ago

I need to investigate the issue first. I will try to find some time over the weekend. If you solve through the innerHTML helper though, please let me know.

Thanks.

johan-ludvigsson-stratsys commented 7 years ago

On a side note, as I read through the source for the innerHTML helper function, it looks like there could be a potential side effect as it calls createdCallback before the children are moved to the upgraded element. Depending on the registered code in the callback, they may make an assumption that there are valid children during createdCallback.

So unless I missunderstand it, shouldn't the children be moved to the new element and after that call createdCallback?

johan-ludvigsson-stratsys commented 7 years ago

The same logic could also apply that the createdCallback references the parentElement, meaning that the createdCallback should be the last thing that occurs.

WebReflection commented 7 years ago

can you eventually file a completely different bug? thank you

johan-ludvigsson-stratsys commented 7 years ago

Sure thing, will see if I can fix a quick test case for it!

javan commented 6 years ago

I ran into the same IE 11 + innerHTML + MutationObserver missing child nodes issue in another project and cooked up a workaround: https://github.com/javan/mutation-observer-inner-html-shim. Sharing in case it's useful here.

php1990 commented 6 years ago

@javan Your fix looks awesome. Thanks a lot for sharing.

WebReflection commented 6 years ago

@javan do you include that before this poly, or after? and do you also use this file at all?

Thanks.

javan commented 6 years ago

@WebReflection, a Google search for the MutationObserver IE 11 bug lead me here, and I haven't actually tested with this poly. I imagine my shim could be included before or after. It just needs to be included before any code that removes nested nodes using innerHTML.

Here's where I'm using it: https://github.com/stimulusjs/stimulus/pull/133

WebReflection commented 6 years ago

@javan a slightly different version of your fix went into 1.8 and there is a link to your original implementation. AFAIK we are both under MIT so I hope that is OK.

I haven't just included your fix because I already have most of the utilities/references in scope + I needed it in scope for the ponyfill anyway.

Please let me know if that's OK with you.

javan commented 6 years ago

Totally 👌

Thanks for asking!

johan-ludvigsson-stratsys commented 6 years ago

Great! Thanks both of you!