Bug 195628 - CSS Variable expression re-evaluated on dom change causing image reload
Summary: CSS Variable expression re-evaluated on dom change causing image reload
Status: RESOLVED DUPLICATE of bug 216048
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 12
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 216048
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-12 10:01 PDT by malsup
Modified: 2020-09-07 05:09 PDT (History)
13 users (show)

See Also:


Attachments
demonstrates issue as well as source html (1.52 MB, video/quicktime)
2019-03-12 10:01 PDT, malsup
no flags Details
Source file to demonstrate. (268 bytes, text/html)
2019-03-18 11:49 PDT, malsup
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description malsup 2019-03-12 10:01:26 PDT
Created attachment 364397 [details]
demonstrates issue as well as source html

Summary:
It appears that Safari is recalculating CSS variables and updating unrelated DOM nodes any time the DOM is changed.  I have created a tiny test page that includes a :hover rule on the body element and a CSS variable for a div which assigns a background image.  Every time I trigger the body's :hover rule it seems to recalculate and update the div's background image (even though the resulting value is unchanged.)  In the case where the image is not cacheable this causes the image to reload every single time the :hover rule activates/deactivates.

Expected Results:
CSS variable evaluation should not have any side effects if the calculated value is unchanged.

Actual Results:
In my example, uncached images are reloaded.

This also happens on iPad (and presumably iPhone).
Comment 1 Radar WebKit Bug Importer 2019-03-15 23:22:40 PDT
<rdar://problem/48950241>
Comment 2 Simon Fraser (smfr) 2019-03-18 11:26:30 PDT
Please attach the source in a non-video form.
Comment 3 malsup 2019-03-18 11:49:37 PDT
Created attachment 365041 [details]
Source file to demonstrate.
Comment 4 Liam DeBeasi 2019-05-15 05:59:51 PDT
I am also running into this issue. I converted the issue author's example into a CodePen to make it a bit easier to access: https://codepen.io/liamdebeasi/pen/QRdjQp

I can confirm this issue is happening on Safari 12 macOS, Mobile Safari on iOS 12.2, and WKWebView on iOS 12.2.

This issue does not happen when the value of CSS variable is a base64 encoded image.
Comment 5 Stephanie (Sullivan) Rewis 2020-02-24 12:48:02 PST
Hi folks, it doesn't appear this bug is getting any traction. This is causing us not to be able to implement important work around web components in an enterprise environment. Do you all have any feedback on when this will be addressed?

Cheers
Comment 6 Jan Miksovsky 2020-05-18 13:19:39 PDT
Simon: You'd asked for the OP to provide a repro in non-video form, which has now been provided.

Can you please assign this for someone to investigate?
Comment 7 Timo 2020-06-01 13:11:38 PDT
Also stuck with this problem. Would be awesome if someone could take a look. Does affect ionic hybrid apps a lot.
Comment 8 Antti Koivisto 2020-06-02 06:33:50 PDT
The issue doesn't repro with the linked codepen anymore (in trunk or STP) because :hover is better optimized and only invalidates the target element.

The underlying issue (uncacheable resources referenced using variables are reloaded on style update) still remains, it can be seen by modifying the test to have :hover and backround on the same element.
Comment 9 Jan Miksovsky 2020-06-02 15:42:48 PDT
To facilitate others looking at this, I've applied Antti's suggested refinement to Liam's repro to reproduce the issue in STP: https://codepen.io/JanMiksovsky/pen/PoZozRw

Antti: Many, many thanks for taking a look at this. Since this issue has once again been confirmed, and continues to affect a range of organizations, could you help get this issue to assigned to someone?
Comment 10 Antti Koivisto 2020-06-03 06:25:12 PDT
Yes, I'm taking a look. Note that we don't really use the assignee field in bugzilla so it is not indicative of anything.
Comment 11 Antti Koivisto 2020-06-03 09:30:49 PDT
This is really about caching policies. The problem reproduces with <img> without any CSS, see https://codepen.io/anttikoivisto/pen/zYrYLwP
Comment 12 malsup 2020-06-03 10:00:25 PDT
Antti, that repo is different in that it's creating new image elements and setting their src attr.  I would expect an image fetch in that case when cache is disabled - that makes perfect sense.

But in the css var example, the value of the background property on the test div is constant.  So it appears that property is getting reevaluated and reset on the element, even though its value is unchanged.  That doesn't sound right.  This would be an issue for any element that rendered non-cacheable assets.
Comment 13 Antti Koivisto 2020-06-04 04:25:01 PDT
CSS variables are evaluated at style resolution time. Side effects like resource loads are retriggered. The behavior difference with other engines seen here is a result of different caching behavior (WebKit respects the uncacheable redirect of the referenced resource). I confirmed this with a Firefox engineer too.

You see a variant of caching-dependent behavior on Chrome too if you open the codepen into two separate windows and hover between them.
Comment 14 Antti Koivisto 2020-06-04 04:26:58 PDT
We may need to change the caching behavior though if this is a web compat issue.
Comment 15 Emilio Cobos Álvarez (:emilio) 2020-06-04 05:22:34 PDT
Note that the document-dependent caching behavior for <img>, at least, is specified in https://html.spec.whatwg.org/#the-list-of-available-images. See the "ignore higher level caching" flag.

It would make sense to me to use the same policy for CSS-loaded images, fwiw.
Comment 16 Antti Koivisto 2020-06-04 08:14:57 PDT
Yeah, makes sense. I didn't know this is specced.
Comment 17 Stephanie (Sullivan) Rewis 2020-07-06 18:22:00 PDT
My team at Salesforce is creating a styling api using CSS custom properties. Due to this bug and another issue where declaring a large number of variables on :root causes a perf issue, we're forced to give Safari static values instead of the customizable experience Chrome and Firefox will receive. Is there any hope in getting Safari's performance in line with other browsers so we don't need to exclude it any longer?
Comment 18 youenn fablet 2020-09-07 05:09:57 PDT
Continuing work in bug 216048

*** This bug has been marked as a duplicate of bug 216048 ***