RESOLVED FIXED 198512
Presence of CSS variable causes a background url() to get resolved with a different base
https://bugs.webkit.org/show_bug.cgi?id=198512
Summary Presence of CSS variable causes a background url() to get resolved with a dif...
Simon Fraser (smfr)
Reported 2019-06-03 19:14:35 PDT
Created attachment 371239 [details] Testcase If the background shorthand has a variable reference in it: background: url('images/ducky.png') var(--darkGreen); then URLs in that shorthand are resolved relative to the document URL, not the style sheet URL, which is wrong.
Attachments
Testcase (245.30 KB, application/zip)
2019-06-03 19:14 PDT, Simon Fraser (smfr)
no flags
Patch (9.69 KB, patch)
2020-10-03 18:01 PDT, Tyler Wilcock
no flags
Patch (9.69 KB, patch)
2020-10-04 16:12 PDT, Tyler Wilcock
no flags
Radar WebKit Bug Importer
Comment 1 2019-06-03 19:14:52 PDT
Simon Fraser (smfr)
Comment 2 2019-06-03 19:22:02 PDT
The CSSParserContext's baseURL is different in the two cases.
Simon Fraser (smfr)
Comment 3 2019-06-03 19:24:29 PDT
...and that's because StyleResolver::resolvedVariableValue() makes a CSSParser parser(document()), so with the document's baseURL, rather than the stylesheet's.
JC Simpson
Comment 4 2020-10-01 23:40:42 PDT
*** Bug 217124 has been marked as a duplicate of this bug. ***
Tyler Wilcock
Comment 5 2020-10-03 18:01:57 PDT
Darin Adler
Comment 6 2020-10-04 14:54:04 PDT
Comment on attachment 410448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410448&action=review > Source/WebCore/css/CSSPendingSubstitutionValue.h:40 > + static Ref<CSSPendingSubstitutionValue> create(CSSPropertyID shorthandPropertyId, Ref<CSSVariableReferenceValue>&& shorthandValue, const URL baseURL) "const URL" is not the type we want for this argument. Could use "const URL&" or "URL". > Source/WebCore/css/CSSPendingSubstitutionValue.h:47 > + const URL baseURL() const { return m_baseURL; } Same here with return value. It can be URL or const URL&, but const URL is not right. > Source/WebCore/css/CSSPendingSubstitutionValue.h:64 > + const URL m_baseURL; Here, the type "const URL" is fine, good, appropriate.
Tyler Wilcock
Comment 7 2020-10-04 16:12:02 PDT
Tyler Wilcock
Comment 8 2020-10-04 17:18:34 PDT
Thanks Darin. I've fixed that and marked the new patch with cq?
EWS
Comment 9 2020-10-04 17:29:03 PDT
Committed r267951: <https://trac.webkit.org/changeset/267951> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410486 [details].
Sam Sneddon [:gsnedders]
Comment 10 2021-04-02 03:54:18 PDT
*** Bug 190807 has been marked as a duplicate of this bug. ***
Vadim Makeev
Comment 11 2021-08-08 15:13:29 PDT
I have just discovered the same problem in my code, here’s the demo: https://bald-ring-chauffeur.glitch.me/ (should be gold) https://bald-ring-chauffeur.glitch.me/subfolder/ (should be green) And here’s the code https://glitch.com/edit/#!/bald-ring-chauffeur It’s a major incompatibility and I’m shocked to see that fix still haven’t shipped. I wonder how soon it’ll make it into Safari, it’s been two years already.
Darin Adler
Comment 12 2021-08-08 15:19:09 PDT
That "same problem" is not fixed by this patch. So either this bug wasn’t successfully fixed 10 months ago, or there is more than one problem with the same effect.
Vadim Makeev
Comment 13 2021-08-08 15:51:29 PDT
(In reply to Darin Adler from comment #12) > That "same problem" is not fixed by this patch. > > So either this bug wasn’t successfully fixed 10 months ago, or there is more > than one problem with the same effect. I’m sorry, but for an outsider it’s hard to tell whether it’s not shipped or it’s not sufficient. Description of this bug and two duplicates looks rather identical to my case. Do you think I should file another bug or someone would reopen the current one? I’m happy to help in any way to get this bug fixed.
Darin Adler
Comment 14 2021-08-08 16:06:14 PDT
I just noticed that we have bug 200092, which might be the problem you are seeing and is not yet fixed. But you can also file a new bug report. That could do some good and should do no harm.
Darin Adler
Comment 15 2021-08-08 16:07:45 PDT
I’m surprised this is not covered in Web Platform Tests; maybe it is and there is a failing test?
Darin Adler
Comment 16 2021-08-08 16:09:01 PDT
(The fix would have been in a Safari Technology Preview release soon after the fix landed on 2020-10-04.)
Darin Adler
Comment 17 2021-08-08 16:10:17 PDT
The patch definitely fixed a bug, it included a test case and that test case was failing and is now passing. We need to figure out what’s different about the new one that is still failing.
Darin Adler
Comment 18 2021-08-08 16:16:15 PDT
OK, studied it a bit more, and yes, the bug Vadim is reporting is bug 200092, not the same as this one. Continuing the discussion there.
Darin Adler
Comment 19 2021-09-14 09:09:07 PDT Comment hidden (obsolete)
Darin Adler
Comment 20 2021-09-14 09:09:45 PDT
(In reply to Vadim Makeev from comment #13) > Do you think I should file another bug or someone would reopen the current > one? I’m happy to help in any way to get this bug fixed. I finally figured out what's wrong and bug 230243 covers fixing it.
Note You need to log in before you can comment on or make changes to this bug.