Polymer: Style node management causes blocking error in ShadowDOM application.

[Westbrook]

The changes found here (https://github.com/Polymer/polymer/commit/819652eb#diff-41695d1bc0c64b86f0e5a7878f58df0f) throw the following error:

Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

When there is an <svg> element with a <style> tag inside of the template of an element that has styles included from a shared source.

Live Demo

http://jsbin.com/lumapiquda/1/edit?html,console,output

Steps to Reproduce

1. Create my-element
2. Add a styled SVG element to that component.
3. Import external styles into that element.
4. Run element

Expected Results

No errors throw.

Actual Results

Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

Browsers Affected

• [x] Chrome
• [ ] Firefox
• [ ] Edge
• [x] Safari 11
• [ ] Safari 8
• [ ] IE 11

Versions

• Polymer: v2.3.0
• webcomponents: master

[LarsDenBakker]

This is affecting our application as well.

[fadijabi]

I am having the same error too.

[TimvdLippe]

Confirmed an issue. I think the queryselector should be changed to :host > style, :host *:not(svg) style, per http://jsbin.com/dafiriquko/1/edit?html,output Assigned @azakus to this issue.

[Westbrook]

The more I review my code in #4976, the more I think the easier answer would be to use something like:

let lastStyle = template.content.firstElementChild;

Obviously, the variable naming _should_ be updated to match, but this covers for more corner cases around <style>s being in unexpected parts of the template without the possibly false expectation that nested <style>s shouldn't have their content run through _processStyleText. AND, it allows for not changing the querySelectorAll at all, which support a clearer path to x-browser support.

[kevinpschaaf]

The real issue is that the processing should only occur on _HTML styles_ and exclude SVG styles, which I think is what @TimvdLippe was angling for. However, the querySelectorAll is being run in the _template_ (before being stamped to a SR), so :host selectors will not work. As noted :scope is not x-plat, so we can't rely on that either. One option is to filter the qSA'ed list of <style>s on namespaceURI (<style> in <svg> will have an SVG namespace).

However, I see a couple additional issues. This code assumes lastStyle will be top-level in the template (we've never enforced that <style>s be top-level afaik):

template.content.insertBefore(s, lastStyle);

To correctly insert it before the last style, the code would need to be changed to this:

lastStyle.parentNode.insertBefore(s, lastStyle);

@azakus BUT, I'm actually confused how the existing logic around lastStyle doesn't result in out-of-order styles (which didn't happen with the previous css text gathering code). Doesn't the current logic result in all but the last concrete styles first, then all included styles next, then the last concrete style? This seems bad if so. Seems like "insert before lastStyle" is too simple, and the code should insert included styles after the previous style. Wdyt?

[kevinpschaaf]

Ok, was slightly wrong there. Confirmed we don't actually need to separate SVG styles from HTML styles; once the lastStyle ordering and inserting logic is fixed the processing will be fine for all styles.

[bennypowers]

I had this same issue affecting an element which contained within it two style tags, one at the top of the template, and one in the middle of the template.

[dfreedm]

Fixed in #4979 and pushed as 2.3.1