Bug 230243 - URLs in CSS variables must be resolved against the base URL of the stylesheet, not the document
Summary: URLs in CSS variables must be resolved against the base URL of the stylesheet...
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
Keywords: InRadar
: 232906 232919 (view as bug list)
Depends on: 198512
Blocks: 200092
  Show dependency treegraph
Reported: 2021-09-13 21:32 PDT by Darin Adler
Modified: 2022-05-09 09:49 PDT (History)
16 users (show)

See Also:

Patch (12.39 KB, patch)
2021-09-13 21:36 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (20.27 KB, patch)
2021-09-14 10:58 PDT, Darin Adler
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2021-09-13 21:32:40 PDT
URLs in CSS variables must be resolved against the base URL of the stylesheet, not the document
Comment 1 Darin Adler 2021-09-13 21:36:32 PDT
Created attachment 438101 [details]
Comment 2 Darin Adler 2021-09-13 21:38:26 PDT
firefoxic.dev@gmail.com contributed a test at https://firefoxic.github.io/test-custom-properties-working-with-url/ but I suspect this is also covered by tests in Web Platform Tests.
Comment 3 Darin Adler 2021-09-13 21:39:13 PDT
Uploaded a first patch to EWS to see what tests have changed results.
Comment 4 Darin Adler 2021-09-14 07:43:03 PDT
No coverage in WPT?!
Comment 5 Simon Fraser (smfr) 2021-09-14 09:34:43 PDT
Did the tests I added here cover this?
Comment 6 Darin Adler 2021-09-14 10:58:16 PDT
Created attachment 438154 [details]
Comment 7 Darin Adler 2021-09-14 11:00:14 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> Did the tests I added here cover this?
> https://github.com/web-platform-tests/wpt/commit/
> 107134aa9334b7ebd7623382c3e19eacab5993d4

Looks like they would, but for some reason I don’t see results changing in those when I run tests.

In the new patch I added to the existing non-WPT test we have for this. Can follow up with additional test coverage.
Comment 8 Simon Fraser (smfr) 2021-09-14 11:18:07 PDT
Those WPT have not been imported yet.
Comment 9 Antti Koivisto 2021-09-14 12:15:04 PDT
Comment on attachment 438154 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=438154&action=review

> LayoutTests/ChangeLog:10
> +        * fast/css/variables/support/styles/url-with-variable-is-sheet-relative.css: Added 10 more test cases.
> +        * fast/css/variables/url-with-variable-is-sheet-relative-expected.html: Ditto.
> +        * fast/css/variables/url-with-variable-is-sheet-relative.html: Ditto.

Would be nice to get these to WPT.
Comment 10 Darin Adler 2021-09-14 12:30:27 PDT
Maybe Simon’s test will be how we cover this in WPT; but I’d also be happy to try to expand the test to cover even more cases. What’s non-obvious is how there are so many subtly different code paths for shorthand vs. longhand and variable references vs. things that are not variables. That’s where the bugs have been hiding.
Comment 11 Darin Adler 2021-09-14 13:00:04 PDT
Committed r282403 (241664@main): <https://commits.webkit.org/241664@main>
Comment 12 Radar WebKit Bug Importer 2021-09-14 13:01:18 PDT
Comment 13 Anthony Ricaud 2021-10-03 05:40:47 PDT
In STP 133 (which should have this fix according to https://webkit.org/blog/11975/release-notes-for-safari-technology-preview-133/), I'm still seeing failures for the longhand test in firefoxic tests.

Should this be reopened?
Comment 14 Darin Adler 2021-10-03 11:44:09 PDT
Please don’t reopen unless you’ve also checked with a nightly build. Should be really easy to download from http://nightly.webkit.org
Comment 15 Darin Adler 2021-10-03 11:44:56 PDT
I will check again with my local build of TOT, but currently waiting on a rebuild.
Comment 16 Anthony Ricaud 2021-10-03 12:39:39 PDT
Unfortunately, I’m on the Monterey beta so no easy nightly build available.
Comment 17 Darin Adler 2021-10-03 18:56:32 PDT
It seems that the Safari Technology Preview release notes are incorrect. I verified that the bug still happens with STP 133 as you said, but I also verified that it works correctly with my locally built WebKit TOT. So there’s some kind of version control or merge problem.
Comment 18 Jon Davis 2021-10-03 19:20:33 PDT
Darin is correct that the STP 133 release notes are wrong. The end revision for STP 133 is incorrect. The list of changes includes many entries that are not part of the build that shipped. The release notes will be updated with the correct set of changes and the corrected end revision number.
Comment 19 Darin Adler 2021-10-03 19:32:53 PDT
Thanks, Jon. Anthony, this was just a couple days after the cutoff point for STP 133. Given that, I expect the fix to be in STP 134.
Comment 20 firefoxic.dev 2021-10-04 13:14:23 PDT
Anyway, Darin, thanks a lot for the fix. I'm really looking forward to the stable Safari release with this fix :)
Comment 21 Anthony Ricaud 2021-10-28 03:47:24 PDT
Great, this is working well in STP 134! Thanks!

(Sadly, this didn't get a shout-out on https://webkit.org/blog/12033/release-notes-for-safari-technology-preview-134/)
Comment 22 Darin Adler 2021-11-10 15:04:31 PST
*** Bug 232919 has been marked as a duplicate of this bug. ***
Comment 23 Darin Adler 2021-11-10 15:04:56 PST
*** Bug 232906 has been marked as a duplicate of this bug. ***