Bug 230243

Summary: URLs in CSS variables must be resolved against the base URL of the stylesheet, not the document
Product: WebKit Reporter: Darin Adler <darin>
Component: CSSAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: 247fetishstore, dm, esprehn+autocc, ews-watchlist, firefoxic.dev, glenn, gsnedders, gyuyoung.kim, heycam, jond, koivisto, macpherson, menard, rik, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 198512    
Bug Blocks: 200092    
Attachments:
Description Flags
Patch
none
Patch koivisto: review+

Darin Adler
Reported 2021-09-13 21:32:40 PDT
URLs in CSS variables must be resolved against the base URL of the stylesheet, not the document
Attachments
Patch (12.39 KB, patch)
2021-09-13 21:36 PDT, Darin Adler
no flags
Patch (20.27 KB, patch)
2021-09-14 10:58 PDT, Darin Adler
koivisto: review+
Darin Adler
Comment 1 2021-09-13 21:36:32 PDT
Darin Adler
Comment 2 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.
Darin Adler
Comment 3 2021-09-13 21:39:13 PDT
Uploaded a first patch to EWS to see what tests have changed results.
Darin Adler
Comment 4 2021-09-14 07:43:03 PDT
No coverage in WPT?!
Simon Fraser (smfr)
Comment 5 2021-09-14 09:34:43 PDT
Darin Adler
Comment 6 2021-09-14 10:58:16 PDT
Darin Adler
Comment 7 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.
Simon Fraser (smfr)
Comment 8 2021-09-14 11:18:07 PDT
Those WPT have not been imported yet.
Antti Koivisto
Comment 9 2021-09-14 12:15:04 PDT
Comment on attachment 438154 [details] Patch 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.
Darin Adler
Comment 10 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.
Darin Adler
Comment 11 2021-09-14 13:00:04 PDT
Radar WebKit Bug Importer
Comment 12 2021-09-14 13:01:18 PDT
Anthony Ricaud
Comment 13 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?
Darin Adler
Comment 14 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
Darin Adler
Comment 15 2021-10-03 11:44:56 PDT
I will check again with my local build of TOT, but currently waiting on a rebuild.
Anthony Ricaud
Comment 16 2021-10-03 12:39:39 PDT
Unfortunately, I’m on the Monterey beta so no easy nightly build available.
Darin Adler
Comment 17 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.
Jon Davis
Comment 18 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.
Darin Adler
Comment 19 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.
firefoxic.dev
Comment 20 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 :)
Anthony Ricaud
Comment 21 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/)
Darin Adler
Comment 22 2021-11-10 15:04:31 PST
*** Bug 232919 has been marked as a duplicate of this bug. ***
Darin Adler
Comment 23 2021-11-10 15:04:56 PST
*** Bug 232906 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.