This manifests as a test failure at <http://w3c-test.org/webapps/ShadowDOM/tests/submissions/Google/tests.html> "Upper-boundary encapsulation: nodes in a shadow DOM subtree are not accessible using the shadow host document's CSSOM extensions" … "assert_equals: style elements in shadow DOM must not be exposed via the document.styleSheets attribute expected 0 but got 1"
Created attachment 143440 [details] Patch
Comment on attachment 143440 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143440&action=review Basically looks OK for me with a few nit comments. > LayoutTests/fast/dom/shadow/document-stylesheets-in-shadow.html:5 > +<script> Why not use LayoutTests/fast/js/resources/js-test-pre.js? > LayoutTests/fast/dom/shadow/document-stylesheets-in-shadow.html:11 > + if (actual == expected) why 2spaces indent here? log has 4 spaces indent though.
Comment on attachment 143440 [details] Patch While I'm no shadow DOM expert, I feel that this patch needs more explanation of what's going on. Why are we skipping over StyleElement::insertedIntoDocument/removedFromDocument? That will not only prevent document.styleSheets from exposing the sheet, but also prevent StyleElement from parsing the sheet, and StyleResolver from applying any of its style (since it gets its sheet data from there.) Maybe <style> elements are supposed to just hang dead inside a shadow subtree?
(In reply to comment #3) > (From update of attachment 143440 [details]) > While I'm no shadow DOM expert, I feel that this patch needs more explanation of what's going on. > > Why are we skipping over StyleElement::insertedIntoDocument/removedFromDocument? That will not only prevent document.styleSheets from exposing the sheet, but also prevent StyleElement from parsing the sheet, and StyleResolver from applying any of its style (since it gets its sheet data from there.) > > Maybe <style> elements are supposed to just hang dead inside a shadow subtree? It's going to be scoped by Shadow DOM, that is, the style is applied only for shadow subtrees. dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#styles I agree Kling's point though. This change could be done as a part of style scoping, instead of just doing a band-aid fix.
Created attachment 144338 [details] Patch
Created attachment 144340 [details] Patch
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 143440 [details] [details]) > > While I'm no shadow DOM expert, I feel that this patch needs more explanation of what's going on. > > > > Why are we skipping over StyleElement::insertedIntoDocument/removedFromDocument? That will not only prevent document.styleSheets from exposing the sheet, but also prevent StyleElement from parsing the sheet, and StyleResolver from applying any of its style (since it gets its sheet data from there.) > > > > > Maybe <style> elements are supposed to just hang dead inside a shadow subtree? > It's going to be scoped by Shadow DOM, that is, the style is applied > only for shadow subtrees. > dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#styles > > I agree Kling's point though. > This change could be done as a part of style scoping, > instead of just doing a band-aid fix. I agree. My old patch made any styles in shadow tree be disabled. For example, <div id='mydiv'></div> ... <script> var div = document.getElementById('mydiv'); var sr = new WebKitShadowRoot(div); sr.innerHTML = '<style>div { background-color: red; }</style><div>RED</div>'; ... </script> The "RED"'s background color is not red.... So I think, I have to modify styleSheets method defined in class Document. I created a new method allStyleSheets which does the same things the old styleSheets does, and modified styleSheets to return just style sheets that are not in shadow trees. Best regards, Takashi Sakamoto
Comment on attachment 144340 [details] Patch As I mentioned, I think this should be attacked as a part of style handling of shadow dom,instead of fixing this locally. http://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#styles Maybe starting point is pulling m_stylesheets from Document up to TreeScope.
Created attachment 144766 [details] Patch
(In reply to comment #8) > (From update of attachment 144340 [details]) > As I mentioned, I think this should be attacked as > a part of style handling of shadow dom,instead of fixing this locally. > http://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#styles > > Maybe starting point is pulling m_stylesheets from Document up to TreeScope. I looked at Document and I felt that it was very difficult to pull m_stylesheets because Document also has m_styleResolver and the style resolver seems to tightly fit Document's stylesheets... So I modified StyleResolver to enforce lower-boundary encapsulation for CSS rules. I also fixed applyAuthorStyles bug, i.e. if applyAuthorStyles is true, all CSS rules declared in ancestors of the shadow DOM subtree will be applied... not only enclosed shadow DOM subtree. I'm not sure whether I'm taking a correct way to fix these bugs. I would like to ask your advice. Best regards, Takashi Sakamoto
This bug has outlived its usefulness.