Bug 226574

Summary: REGRESSION (r276882): custom properties not available on host on initial paint
Product: WebKit Reporter: vb
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: bfulgham, koivisto, sam, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

vb
Reported 2021-06-03 02:41:58 PDT
With multiple shadow doms sharing a stylesheet their custom properties defined on the :host element are not available on first paint. This leads to incorrect layout on first paint. When hovering over such an element a repaint occurs where the custom properties are now available and a flash occurs. When there is only one shadow dom the problem does not exist. A reproduction can be visited under https://realityfilter.github.io/safari-custom-properties-regression/ The github repo is https://github.com/realityfilter/safari-custom-properties-regression Sometimes you have to clear the caches before reloading the page to see this bug. The current Safari 14.4.1 does not have this bug. This should have a huge impact on current frameworks like ionic, although I have not tested it, yet. Simple external CSS: ```css :host, :root { --background-color: red; } div { background-color: var(--background-color); border: 1px solid black; } ``` Javascript: ```js const attachApp = (element) => { const root = element.attachShadow({mode: "open"}) const style = document.createElement('link') style.rel= 'stylesheet' style.href = './app.css' root.append(style) const div = document.createElement('div') const p = document.createElement('p') p.textContent = 'I should have a red background' div.append(p) root.append(div) } document.querySelectorAll('.app').forEach(attachApp) ``` ```html <head> </head> <body> <!-- only one element is fine --> <div class="app"></div> <!-- with a second one, styling is incorrect --> <div class="app"></div> <script type="module" src="./app.js"></script> </body> ```
Attachments
patch (5.54 KB, patch)
2021-06-04 08:40 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
vb
Comment 1 2021-06-03 02:43:53 PDT
I think the bug was introduced in Safari Technology Preview 125.
Radar WebKit Bug Importer
Comment 2 2021-06-04 04:10:55 PDT
<rdar://problem/78863643>
Antti Koivisto
Comment 3 2021-06-04 04:15:44 PDT
Looks like a style invalidation issue with external stylesheets in a shadow tree.
vb
Comment 4 2021-06-04 04:29:15 PDT
We noticed it on a larger project. Some parts of the stylesheet are applied. E.g the black border in the example is in place even on first paint.
Antti Koivisto
Comment 5 2021-06-04 05:00:09 PDT
Right, it looks like variables are not being inherited to the shadow tree correctly.
Antti Koivisto
Comment 6 2021-06-04 07:47:50 PDT
The issue is that we fail to invalidate the shadow host style when switching from empty shared style resolver to non-empty one. Variables set on the host won't be resolved and fail to inherit to the shadow trees.
Antti Koivisto
Comment 7 2021-06-04 08:40:09 PDT
Created attachment 430581 [details] patch
Simon Fraser (smfr)
Comment 8 2021-06-04 10:38:15 PDT
Comment on attachment 430581 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=430581&action=review > Source/WebCore/style/StyleInvalidator.cpp:393 > + if (!resolver) > + return true; Would it be simpler to do the !resolver check and invalidations up-front?
Antti Koivisto
Comment 9 2021-06-04 11:19:14 PDT
> Would it be simpler to do the !resolver check and invalidations up-front? I suppose the invalidation could go to helper lambdas but I feel this is pretty readable as-is.
Antti Koivisto
Comment 10 2021-06-04 11:19:59 PDT
Thanks for a good test case vb@bigdot.de!
EWS
Comment 11 2021-06-04 11:52:34 PDT
Committed r278478 (238496@main): <https://commits.webkit.org/238496@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430581 [details].
Note You need to log in before you can comment on or make changes to this bug.