| 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: | CSS | Assignee: | 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
Darin Adler
2021-09-13 21:32:40 PDT
Created attachment 438101 [details]
Patch
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. Uploaded a first patch to EWS to see what tests have changed results. No coverage in WPT?! Did the tests I added here cover this? https://github.com/web-platform-tests/wpt/commit/107134aa9334b7ebd7623382c3e19eacab5993d4 Created attachment 438154 [details]
Patch
(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. Those WPT have not been imported yet. 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. 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. Committed r282403 (241664@main): <https://commits.webkit.org/241664@main> 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? Please don’t reopen unless you’ve also checked with a nightly build. Should be really easy to download from http://nightly.webkit.org I will check again with my local build of TOT, but currently waiting on a rebuild. Unfortunately, I’m on the Monterey beta so no easy nightly build available. 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. 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. 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. Anyway, Darin, thanks a lot for the fix. I'm really looking forward to the stable Safari release with this fix :) 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/) *** Bug 232919 has been marked as a duplicate of this bug. *** *** Bug 232906 has been marked as a duplicate of this bug. *** |