WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Patch
(9.69 KB, patch)
2020-10-03 18:01 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Patch
(9.69 KB, patch)
2020-10-04 16:12 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2019-06-03 19:14:52 PDT
<
rdar://problem/51379244
>
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
Created
attachment 410448
[details]
Patch
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
Created
attachment 410486
[details]
Patch
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)
(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.
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.
Top of Page
Format For Printing
XML
Clone This Bug