Custom elements containing a shadow root with a style tag containing an @import statement causes any following elements to not render
Reported by
brianchu...@gmail.com,
Nov 13
|
||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36 Steps to reproduce the problem: 1. Create a custom element with a shadow root and insert a style tag with an @import statement (the actual css in the path does not have to resolve) 2. Add custom element to the page and any regular element following it What is the expected behavior? The custom element and the built in elements should render What went wrong? Any elements following the custom element in the dom are not rendered on the page. This is resolved if the `this.shadow.appendChild` calls are put in a setTimeout or if the <script> containing the custom element definition is placed after the usage of the tag. Did this work before? Yes 66 Does this work in other browsers? Yes Chrome version: 70.0.3538.102 Channel: stable OS Version: OS X 10.13.6 Flash Version: Chrome v66 just happened to be an older version of chrome that i had available. it might have been working in newer versions than that.
,
Nov 14
Able to reproduce the issue on reported chrome version 70.0.3538.102 and on the latest canary 72.0.3608.0 using Ubuntu 14.04, Windows 10 and Mac 10.13.1 Bisect Information: ------------------- Good Build: 69.0.3442.0 Bad Build: 69.0.3443.0 You are probably looking for a change made after 562149 (known good), but no later than 562150 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/883c397ddd952ca243d37f7ce68dc3b4235f2f20..21154794b8ae44ef571979016fd64b1de23a7cb7 Suspecting: https://chromium.googlesource.com/chromium/src/+/21154794b8ae44ef571979016fd64b1de23a7cb7 Review URL: https://chromium-review.googlesource.com/1072308 @Patrick Meenan: Please help in assigning it to the right owner, if this isn't related to your change. Thanks!
,
Nov 14
,
Nov 21
The testcase in #0 appends the stylesheet to the shadowRoot, but the same bug will occur if it is appended to the main document's body. This bug involves a corner case of the HTML parser that was previously overlooked. Current behavior, after the patch mentioned in #2, is that style sheets added while parsing the document after the first <body> tag block the parser from proceeding further. However, in this case, there is a createdCallback script being called synchronously from within the creation of the custom element, and in the createdCallback it adds a stylesheet with an external dependency. This causes the HTMLDocumentParser to go into IsPaused mode, while consuming a string of tokens for elements. This then leads to this DCHECK hitting: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/parser/html_document_parser.cc?q=html_document_parser.cc&sq=package:chromium&dr&l=533 // The script or stylesheet should be the last token of this bunch. DCHECK_EQ(it + 1, tokens.end()); The code appears to be assuming that pausing can only happen during evaluation of a <style> or <script> block, which is apparently put into its own token chunk, whereas custom elements are handled more generally. Kouhei, could you take a look and advise about how to fix this? I think the correct behavior should be for the parsing of the custom element's closing tag to finish, then the parser should stop and re-start with subsequent tokens. Not sure how hard this is to do.
,
Nov 21
Edit: "re-start with subsequent tokens once the stylesheet loads"
,
Nov 21
c#4: I think we should only block parser iff <link rel=stylesheet> tag was parser-inserted by the main document parser. The code in question seems to be blocking the main document parser on script inserted elements, which seems bad. Talking to tkent@, we might want to consider reverting the change (temporarily) if this bug is actually P1.
,
Nov 21
It seems the culprit CL caused another regression; Issue 882287 .
,
Nov 21
It sounds we should revert the culprit CL, disabling the flag. Both bugs sound P1 regression to me.
,
Nov 22
This feature has been in the stable channel for more than one release milestone - it launched in M69. I do not think we should revert it. I will think some more about your proposal to only block the parser on parser-inserted stylesheets. Maybe that is a good idea...
,
Nov 24
Ok I agree that stopping the parser due to script-inserted stylesheets is incorrect, and will fix it.
,
Nov 24
Issue 882287 has been merged into this issue.
,
Nov 26
Now I'm not so sure about my statement in #10. For example, a custom element's shadow tree currently cannot be parser-inserted, yet it participates in the parsing lifecycle by means of its connectedCallback. Also, custom elements can have their own style sheets, which if parented below their shadowRoot, apply only to themselves. Thus the only way to actually style a custom element's shadow DOM is via imperative code, which is not parser-inserted. Yet custom elements should be able to participate in the concept of render-blocking stylesheets. Therefore we should allow connectedCallback scripts to insert stylesheets which block parsing.
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17f95119ef504da028a2ad2170e44ec0a62305e8 commit 17f95119ef504da028a2ad2170e44ec0a62305e8 Author: Chris Harrelson <chrishtr@chromium.org> Date: Tue Nov 27 21:05:53 2018 Force a new token chunk after custom element start tags. This ensures that that token is always at the end of a token chunk. This is important because the parser can only yield or pause at token chunk boundaries. Pausing may be forced by a custom element because its createdCallback routine may add a stylesheet, which blocks the HTML parser. Bug: 904966 Change-Id: I9252067f435d048b860cc9725ba23d829c9f644d Reviewed-on: https://chromium-review.googlesource.com/c/1351366 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#611317} [modify] https://crrev.com/17f95119ef504da028a2ad2170e44ec0a62305e8/third_party/blink/renderer/core/html/parser/background_html_parser.cc [modify] https://crrev.com/17f95119ef504da028a2ad2170e44ec0a62305e8/third_party/blink/renderer/core/html/parser/html_tree_builder_simulator.cc [modify] https://crrev.com/17f95119ef504da028a2ad2170e44ec0a62305e8/third_party/blink/renderer/core/html/parser/html_tree_builder_simulator.h [add] https://crrev.com/17f95119ef504da028a2ad2170e44ec0a62305e8/third_party/blink/web_tests/fast/parser/stylesheet-added-in-custom-element-during-parse-expected.html [add] https://crrev.com/17f95119ef504da028a2ad2170e44ec0a62305e8/third_party/blink/web_tests/fast/parser/stylesheet-added-in-custom-element-during-parse.html
,
Nov 27
,
Nov 27
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by tkent@chromium.org
, Nov 14Labels: Needs-Bisect
Owner: rakina@chromium.org
Status: Assigned (was: Unconfirmed)