Created attachment 290231 [details] Demo Mutating StyleSheet object by calling insertRule, disabling style element itself, etc... doesn't work inside a shadow tree.
<rdar://problem/28550588>
Created attachment 290812 [details] patch
Created attachment 290814 [details] patch
Comment on attachment 290814 [details] patch Attachment 290814 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2231217 New failing tests: fast/dom/StyleSheet/detached-parent-rule-without-wrapper.html
Created attachment 290817 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290814 [details] patch Attachment 290814 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2231220 New failing tests: fast/dom/StyleSheet/detached-parent-rule-without-wrapper.html
Created attachment 290818 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 290814 [details] patch Attachment 290814 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2231225 New failing tests: fast/dom/StyleSheet/detached-parent-rule-without-wrapper.html
Created attachment 290819 [details] Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 290814 [details] patch Attachment 290814 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2231222 New failing tests: fast/dom/StyleSheet/detached-parent-rule-without-wrapper.html
Created attachment 290820 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 290846 [details] patch
Comment on attachment 290846 [details] patch Attachment 290846 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2232943 New failing tests: fast/shadow-dom/mutating-stylesheet-in-shadow-tree.html
Created attachment 290853 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290846 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=290846&action=review It looks like two test cases are failing on WK1? > Source/WebCore/css/CSSStyleSheet.cpp:173 > + auto* scope = styleSheetScope(); I'm not sure scope is the most descriptive name of AuthorStyleSheets objects but perhaps you're planning to rename it? > Source/WebCore/css/CSSStyleSheet.cpp:-205 > - owner->authorStyleSheets().didChangeCandidatesForActiveSet(); Could you clarify in the change log why it's okay to remove this call? > Source/WebCore/css/CSSStyleSheet.cpp:410 > +{ > + return const_cast<CSSStyleSheet&>(*this).rootStyleSheet(); > +} Maybe we can just inline this in the header? > Source/WebCore/dom/AuthorStyleSheets.cpp:88 > +AuthorStyleSheets& AuthorStyleSheets::forNode(Node& node) It's pretty neat that we just needed three functions to abstract Document/ShadowRoot difference! > Source/WebCore/dom/AuthorStyleSheets.cpp:337 > + if (m_shadowRoot) > + updateType = UpdateType::ContentsOrInterpretation; > + Why do we want to override this? It seems that we do want to analyzeStyleSheetChange even inside a shadow tree? Please clarify that in the change log. > Source/WebCore/html/HTMLLinkElement.cpp:-333 > - > - if (document().hasLivingRenderTree()) > - document().authorStyleSheets().didChangeCandidatesForActiveSet(); Could you clarify why it's okay to remove this in the change log?
Created attachment 290854 [details] patch
> I'm not sure scope is the most descriptive name of AuthorStyleSheets objects > but perhaps you're planning to rename it? Yes, in a separate patch.
> Why do we want to override this? It seems that we do want to > analyzeStyleSheetChange even inside a shadow tree? We don't currently do optimized updates in shadow trees which is not changed by this patch. I'll enable it later.
Created attachment 290857 [details] patch
Comment on attachment 290857 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=290857&action=review r=me assuming tests pass on WK1 > Source/WebCore/dom/AuthorStyleSheets.cpp:337 > + if (m_shadowRoot) > + updateType = UpdateType::ContentsOrInterpretation; Should we add a FIXME saying that we should eventually be able to remove this special case?
https://trac.webkit.org/r206880