Created attachment 284846 [details] Safari 10 left, Chrome 54 right I'm running macOS Sierra and using Safari 10. I'm using a custom element polyfill for v1 along with native Shadow DOM v1 and no styles are getting picked up at all. I've added style tags to the shadow roots. Things work fine in Chrome Canary (native v1). See http://skate.js.org and inspect the shadow roots to debug it. I've attached a side-by-side screenshot of Safari 10 and Chrome 54 running the same page. I've raised this as a blocker because if it is in fact an issue, Shadow DOM is unusable. I also raised this under CSS because there is no component for Shadow DOM or Web Component it seems. Thanks!
Wow, this is a terrible bug. It looks like something is style recalc is completely broken. e.g. if you re-insert the style element again, it renders correctly. The bug here is that setting style element's content doesn't update the element. You can work around it by re-inserting the element in your `applyStyle` function: https://github.com/skatejs/kickflip/blob/8dd73b87c7027acb7b3da4906c9a290569a4c42e/dist/index.js#L525 as in el.parentNode.insertBefore(el, el.nextSibling);
<rdar://problem/27609715>
The problem here appears to be that Document::styleResolverChanged doesn't update style resolvers in shadow trees.
Or that we don't have ShadowRoot equivalent of styleResolverChanged.
(In reply to comment #4) > Or that we don't have ShadowRoot equivalent of styleResolverChanged. Adding that would solve this problem. It looks like we're missing a bunch of style recalcs due to media type changes, etc... right now though. I think we need some mechanism to notify all style resolvers in a given document to be updated for those.
Just wondering if there's been any movement on this. We're still seeing this in Safari 10 stable, which renders native Shadow DOM V1 unusable for us. Thanks!
(In reply to comment #6) > Just wondering if there's been any movement on this. We're still seeing this > in Safari 10 stable, which renders native Shadow DOM V1 unusable for us. Fixing this bug requires solving some architectural problem in WebKit but we're hoping to fix in the coming months.
The main reason this fails is a simple bug. We don't signal text content mutations to parent in shadow trees: void CharacterData::dispatchModifiedEvent(const String& oldData) { ... if (!isInShadowTree()) { if (parentNode()) { ... parentNode()->childrenChanged(change); } Fixing this makes the page render mostly correctly. Generally we should have a reduced test case available before trying to analyze the cause.
...or at least before pinning them to deep architectural issues. :)
Created attachment 289818 [details] patch
Comment on attachment 289818 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=289818&action=review > Source/WebCore/dom/CharacterData.h:62 > + void notifyParentAfterChange(ContainerNode::ChildChangeSource); Does this really need to be a protected member? Can't it be a private member instead?
Comment on attachment 289818 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=289818&action=review > Source/WebCore/dom/AuthorStyleSheets.cpp:343 > - if (m_shadowRoot) > - m_shadowRoot->setNeedsStyleRecalc(); > - else > + if (m_shadowRoot) { > + for (auto& shadowChild : childrenOfType<Element>(*m_shadowRoot)) > + shadowChild.setNeedsStyleRecalc(); > + } else It seems easy to make this kind of mistake and call setNeedsStyleRecalc() on a ShadowRoot. Would it make sense to either override ShadowRoot::setNeedsStyleRecalc() and have it call Element::setNeedsStyleRecalc() on its children or mark ShadowRoot::setNeedsStyleRecalc() as "delete"d to cause a compile-time error when ShadowRoot::setNeedsStyleRecalc() is called.
(In reply to comment #9) > ...or at least before pinning them to deep architectural issues. :) I think we still have an issue where we don't trigger style recalc in every shadow tree in Document::setContentLanguage, Page::setViewMode, Page::setNeedsRecalcStyleInAllFrames, SVGFontFaceElement::rebuildFontFace, and SVGFontFaceElement::removedFrom to name a few.
Also FrameView::setViewportSizeForCSSViewportUnits and possibly FrameView::setPagination.
Sure, there are some known and likely many unknown shadow DOM related bugs and inefficiencies.
(In reply to comment #15) > Sure, there are some known and likely many unknown shadow DOM related bugs > and inefficiencies. Well, my point about the architectural problem is that basically everywhere we call Document::styleResolverChanged is a place where we might need to find a StyleResolver in every shadow tree and trigger a similar style recalc logic.
https://trac.webkit.org/r206404
> It seems easy to make this kind of mistake and call setNeedsStyleRecalc() on > a ShadowRoot. Would it make sense to either override > ShadowRoot::setNeedsStyleRecalc() and have it call > Element::setNeedsStyleRecalc() on its children or mark > ShadowRoot::setNeedsStyleRecalc() as "delete"d to cause a compile-time error > when ShadowRoot::setNeedsStyleRecalc() is called. Good idea, will do something like that later.
For posterity, el.parentNode.insertBefore(el, el.nextSibling) doesn't seem to work. I had to first remove the style node and then insert before the next sibling. Our solution in its entirety was to override attachShadow() and observe the shadow root that was created so that we can observe any style elements in it and re-insert them if they were mutated. Code for fix: https://bitbucket.org/atlassian/atlaskit/src/c4fe7f74539ca5e76449c1c72b960ad0c0809fd0/packages/akutil-polyfills/src/index.js?fileviewer=file-view-default We're going to go ahead with the override for a short-term fix. Thanks again for looking into this and fixing it so quickly.