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.
<rdar://problem/51379244>
The CSSParserContext's baseURL is different in the two cases.
...and that's because StyleResolver::resolvedVariableValue() makes a CSSParser parser(document()), so with the document's baseURL, rather than the stylesheet's.
*** Bug 217124 has been marked as a duplicate of this bug. ***
Created attachment 410448 [details] Patch
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.
Created attachment 410486 [details] Patch
Thanks Darin. I've fixed that and marked the new patch with cq?
Committed r267951: <https://trac.webkit.org/changeset/267951> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410486 [details].
*** Bug 190807 has been marked as a duplicate of this bug. ***
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.
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.
(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.
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.
I’m surprised this is not covered in Web Platform Tests; maybe it is and there is a failing test?
(The fix would have been in a Safari Technology Preview release soon after the fix landed on 2020-10-04.)
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.
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.
(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 200092 covers fixing it.
(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.