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

Description vb 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>
```
Comment 1 vb 2021-06-03 02:43:53 PDT
I think the bug was introduced in Safari Technology Preview 125.
Comment 2 Radar WebKit Bug Importer 2021-06-04 04:10:55 PDT
<rdar://problem/78863643>
Comment 3 Antti Koivisto 2021-06-04 04:15:44 PDT
Looks like a style invalidation issue with external stylesheets in a shadow tree.
Comment 4 vb 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.
Comment 5 Antti Koivisto 2021-06-04 05:00:09 PDT
Right, it looks like variables are not being inherited to the shadow tree correctly.
Comment 6 Antti Koivisto 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.
Comment 7 Antti Koivisto 2021-06-04 08:40:09 PDT
Created attachment 430581 [details]
patch
Comment 8 Simon Fraser (smfr) 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?
Comment 9 Antti Koivisto 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.
Comment 10 Antti Koivisto 2021-06-04 11:19:59 PDT
Thanks for a good test case vb@bigdot.de!
Comment 11 EWS 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].