Correctly resolve relative URLs for registered CSS custom properties. Relative URLs in registered custom properties must resolve against the base URL of the originating stylesheet. Currently they resolve against the base of the document. For instance, consider a stylesheet at /style/mystyle.css: --foo: url("myimage.jpg"); And a document at /index.html: <style> background-image: var(--foo); </style> If the property --foo is registered with syntax <url>, then the background-image should be /style/myimage.jpg. This is contrary to non-registered properties, in which case the background-image would be /myimage.jpg. This used to be a bug in Chromium too, but it has been fixed: https://bugs.chromium.org/p/chromium/issues/detail?id=851490 I've borrowed the description from that bug report, as it describes the issue clearly.
Dup of bug 198512?
<rdar://problem/53566173>
<rdar://problem/53566174>
(In reply to Simon Fraser (smfr) from comment #1) > Dup of bug 198512? No, not the same thing. Confusingly similar. The WPT test that covers this is: css/css-properties-values-api/url-resolution.html
I’ll take a look at this.
Looked over the Chromium fix for this and the approach doesn’t seem quite like what we’d want. The value is expanded from a relative URL to an absolute URL as part of the token expansion process. I think we can find a way to make sure we pass through the proper base when the value is parsed without doing that, but not sure.
One interesting question is what cssText should return for such properties? Should it have the original relative URL or should it have the resolved absolute URL?
I think the key is the places that complete the URL; they need to get the original context for the URL's tokens rather than using the URL of the rule currently being parsed. Those places are: CSSPropertyParserHelpers::consumeImage CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrcURI These are the only two places where this resolution is done. We need to figure out a way to include the context for these tokens when the variable is expanded. Obviously we don’t want every single CSSParserToken to carry this context, but I think we can find a way to carry it when needed. One way might one to teach CSSParserContext to understand the different context for various stretches of variables? Also unsure if we need to support RawStringAsURL or just URLFunction.
The RawStringAsURL case is for image sets. My guess is that they won’t work correctly in the other browsers, because the var value in that case wouldn’t even need to contain "url()". Definitely didn’t see code that would correctly handle that in the Chromium patch. Not sure: will have to see if I can construct a test case. Not all that familiar with image set syntax.
The behavior that Chromium implemented and that this bug asks for is a nice idea, but is not specified in the current CSS draft. See this: https://drafts.csswg.org/css-variables-1/#syntax That section contains a note that says: For example, relative URLs in CSS are resolved against the base URL of the stylesheet the value appears in. However, if a custom property like --my-image: url(foo.jpg); shows up in an "/a/style.css" stylesheet, it will not resolve into an absolute URL immediately; if that variable is later used in a different "/b/style.css" stylesheet like background: var(--my-image);, it will resolve at that point to "/b/foo.jpg". Given that comment, I am not sure whether to just "do what Chromium does" or follow the CSS specification. Before I start coding we probably need to clear this up. Would like to know if Firefox has also made this change. Maybe someone could test there?
> Would like to know if Firefox has also made this change. Maybe someone could test there? It works the same way both in Firefox and Chrome. I’m afraid it’s too late to change that, since there are two independent implementations.
(In reply to Vadim Makeev from comment #11) > I’m afraid it’s too late > to change that, since there are two independent implementations. Not sure that’s right. Why didn’t we change the CSS specification when we started changing the browsers? Someone needs to raise the issue and get the CSS specification fixed.
One of the problems with not having this defined in CSS is that it’s likely messed up for image-set and not even defined what the behavior should be for that case, which has a different syntax.
My regards! > Would like to know if Firefox has also made this change. Maybe someone could test there? I have prepared a test based on the behavior described in that note in the specification: https://firefoxic.github.io/test-custom-properties-working-with-url/ > Why didn’t we change the CSS specification when we started changing the browsers? As the test shows, Firefox and Chromium behave as described in the specification, and even expected by web developers. I hope the test will help in resolving the issue. If I missed something, I am ready to correct it promptly.
(In reply to firefoxic.dev from comment #14) > My regards! > > > Would like to know if Firefox has also made this change. Maybe someone could test there? > > I have prepared a test based on the behavior described in that note in the > specification: > https://firefoxic.github.io/test-custom-properties-working-with-url/ Thanks! Is that test really consistent with what the note says? Maybe I am misreading the specification, but I thought the note says it should *not* do that.
(In reply to Darin Adler from comment #15) > Is that test really consistent with what the note says? Maybe I am misreading the specification, but I thought the note says it should *not* do that. Many thanks to Ilya Streltsyn, who figured out this issue. He found the confusion in the first message. > If the property --foo is registered with syntax <url>, then the background-image should be /style/myimage.jpg. > This is contrary to non-registered properties, in which case the background-image would be /myimage.jpg. It seems that this is what confused you. After all, the opposite is true: if a custom property is not registered, then the path must be resolved at the place of application, not the property definition because such property is passed as a token; but the registered custom property is passed not as a token, but as an already processed value, in which case the path should be resolved at the place of definition immediately. Since this bug and my test are both not about registered properties, the test is built correctly. Well, a failure of such a variant as “outside”, when a custom property is simply used somewhere separated by a comma and does not contain a path — in any case, it is not correct. Especially considering that the behavior for all “shorthand” variants has already been fixed.
Does WPT contain tests both for the registered custom property case and the unregistered custom property case?
Can someone help me find the text that explains this difference between registered <url> and non-registered properties in the standards documents? That will help me understand precisely how to do this.
Here’s something I haven’t figured out yet that is delaying my work to fix this bug: At the moment now I can’t find any code in WebKit that parses syntax definitions. The only thing I see is a check if the syntax string is literally "*". The WPT test for this, css/css-properties-values-api/url-resolution.html registers a property with the syntax "<url> | none". I don’t know if the URL resolution behavior depends on the property being registered with syntax "<url>" or if, alternatively, any URL encountered as the value of any registered property should be resolved this way, regardless of syntax. Not sure how to construct a test that could tell the difference in this. I suppose this confusion is partly because I am so new to the CSS registered properties feature. Very difficult to understand it! So I don’t yet know if I first have to implement registered property syntax parsing before tackling this bug. Or if it’s OK for now to go on not parsing the registered property syntax, because we can possibly handle all URLs in any registered property this way regardless of syntax. Which seems peculiar but maybe it’s correct. Is it syntax independent? Is "*" a special case for this URL resolution? Pretty sure the WPT tests in the css/css-properties-values-api directory do not cover this since custom properties are all registered with syntax "<url> | none", so there’s no test that can tell how the syntax string affects this.
Also, since I can’t fully understand the specification, I can’t yet use the specification language to resolve this ambiguity. The specification talks about "<url>" but I can’t yet tell if that’s just referring to the type of the value or tokens or talking about the registered syntax.
(In reply to firefoxic.dev from comment #14) > As the test shows, Firefox and Chromium behave as described in the > specification, and even expected by web developers. I think this is confusing. Firefox doesn't implement the properties-and-values api, so we have no concept of registered properties yet. Or is this not specific to registered properties as the title and other bits imply?
I’d love to work on this next, but right now I’m not clear enough on what the requirements are.
I think there are two bugs here. One interop issue with regular custom properties where WebKit behaves incorrectly (comment 14), and the different registered custom property behavior which is specific to properties-and-values (comment 0). It's probably worth splitting those up (and I suspect comment 14 is more urgent since iirc WebKit is not shipping properties and values).
I agree. Could someone write up the thing from comment 14? I am particularly interested in where and how the specification calls for this; it seems the implementers on Firefox and Chrome didn’t have any problem understanding it, but it is proving difficult for me. I still think the specification says the opposite of what the browsers have implemented!
(In reply to Darin Adler from comment #24) > I still think the specification says the > opposite of what the browsers have implemented! How so? In the example of comment 14, the background-image declaration is in ./b/style.css, so background-image: var(--my-image) resolves to ./b/foo.svg, which is the "Success!" image. That's the behavior in comment 10, for unregistered properties. It seems WebKit is using the document base URI rather than the stylesheet's base uri to resolve the image, which is clearly wrong. Now if in Chrome you run this on the console: CSS.registerProperty({ name: "--my-image", syntax: "<url>", inherits: false, initialValue: "url(about:invalid)" }) Then you get two "Wrong folder" images. That's the behavior from comment 0, but that's tangential to comment 14.
For reference, the way we implement it in Firefox is keeping the stylesheet's url data in the unparsed value: https://searchfox.org/mozilla-central/rev/fdcee847713a33b5e2b4175bb3c72d4aa2215515/servo/components/style/properties/properties.mako.rs#1647-1658
You can completely ditch the differences between registered and unregistered custom properties and just take a look at the property: background-image: url("./foo.svg"), linear-gradient(var(--my-color), var(--my-color)); Obviously, the path to the image in this property has absolutely nothing to do with custom properties and must be resolved relative to the style file (i.e. '/b/foo.svg'), but it is resolved (in the latest WebKit) relative to the markup file (i.e. '/ foo.svg '). I absolutely cannot understand how the mention of a custom property (no matter which one) somewhere separated by a comma in another part of the multiple background value affects the explicitly specified path to the image. At the same time, if you replace the longhand with a shorthand: background: url("./foo.svg"), linear-gradient(var(--my-color), var(--my-color)); then everything works as expected (thanks for fixing bug #198512).
Thanks for the help figuring this out. So if I understand correctly, there is a something wrong in WebKit that is *not* the problem originally described on comment #0. This thing is already correct in both Firefox’s engine and Chromium, and everyone except me thinks it’s pretty clearly explained correctly already in the relevant standards. We should make a new separate bug report about that. And I plan to work to fix that bug. But this bug should remain, tracking the issue from comment #0, which, for example, apparently doesn’t even apply yet to Firefox since there is no registration of CSS custom properties in the Firefox engine yet. Does that seem accurate to everyone else?
Wrote a new bug 230243 for the problem I am fixing first.
Sorry I was so slow to understand, I get it now.
Back to the original question: Can someone point me to the specification where this behavior for URLs in registered custom CSS properties is defined?
I think I found it. Substitution of registered custom properties says it must be replaced with an "equivalent token sequence": https://www.w3.org/TR/css-properties-values-api-1/#substitution Those are based on computed values, and computed values for "<url"> are the resolved absolute URL: https://www.w3.org/TR/css-properties-values-api-1/#calculation-of-computed-values So I think the starting point is to make sure that our CSS code tries to use computed values and equivalent token sequences for registered properties.
Note, this bug does not yet affect the Safari web browser. It is a bug in the CSS Custom Values and Properties API, which is a Safari experimental feature, not yet complete, and currently off by default. That is part of what confused me for so long. My test pages weren’t working at all.
This is only a tiny part of what is currently missing from our implementation of CSS Properties and Values API. When someone works on that, they should definitely fix this along with the rest of the specification. Currently we don’t even look at the syntax of custom properties, except for checking if it is "*" or not. Fixing this bug requires checking the syntax for "<url>". I did briefly look at the Chromium patch and it does not check the syntax, so I don’t think it matches what the specification calls for, so I think there’s more than just "fix this bug". I think we need to work on some tests. Now that I know this isn’t just one specific bug, I’m not going to be working on this further right now. I will help whoever ends up working on completing our implementation of CSS Properties and Values API to get this right. If you are that person, please get in touch!
WPT test case for URL Resolution: https://wpt.fyi/results/css/css-properties-values-api/url-resolution.html?label=experimental&label=master&aligned= ^ Safari Technical Preview 175 is passing all tests.
Resolving as per Ahmad's comment.