New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CSS2] empty url() behaviour needs errata to match Level 3 #2211

Open
gsnedders opened this Issue Jan 21, 2018 · 12 comments

Comments

Projects
None yet
5 participants
@gsnedders
Contributor

gsnedders commented Jan 21, 2018

https://drafts.csswg.org/css-values-3/#url-empty states:

If the value of the url() is the empty string (like url("") or url()), the url must resolve to an invalid resource (similar to what the url about:invalid does).

This contradicts CSS2, which states:

In order to create modular style sheets that are not dependent on the absolute location of a resource, authors may use relative URIs. Relative URIs (as defined in [RFC3986]) are resolved to full URIs using a base URI. RFC 3986, section 5, defines the normative algorithm for this process. For CSS style sheets, the base URI is that of the style sheet, not that of the source document.

As an empty URL is a relative URI as defined by RFC3986, per CSS2 if the value of the url() is the empty string, then it must resolve to the URL of the style sheet.

This is observable both through the CSSOM and through use of content negotiation and polyglot documents.

@gsnedders

This comment has been minimized.

Contributor

gsnedders commented Jan 21, 2018

(FWIW, credit to web-platform-tests/wpt#9113 (comment) for making me notice this discrepancy.)

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 3, 2018

@gsnedders Can you clarify what behavior is currently implemented? Because we should align both specs to that. (Assuming it's one or the other, and not something else completely insane.)

@fantasai fantasai added the Agenda+ label Feb 3, 2018

@gsnedders

This comment has been minimized.

Contributor

gsnedders commented Feb 3, 2018

FWIW:

This was added in e051892, per a WG resolution, so we'd presumably have to revert the resolution and then maybe eventually issue errata for 2.1 and 3 if we still want to change this behaviour in 4.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 6, 2018

@gsnedders I can't seem to find the minutes on this issue. :( But also, can you answer the above question? :)

@FremyCompany

This comment has been minimized.

Contributor

FremyCompany commented Feb 13, 2018

Edge does not parse url() as a valid url (by choice, visibly).
Chrome and Firefox seems to.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 14, 2018

@FremyCompany That seems odd. What about url('')?

@FremyCompany

This comment has been minimized.

Contributor

FremyCompany commented Feb 14, 2018

@fantasai I apparently need to correct my previous statement. Both url are valid at parse time in Edge, we just incorrectly serialize them as 'none' in background-image. If I had used content I would have seen it work.

The default value of the flag isURL = false I saw in the code is irrelevant because its value is always set to true when you encounter the closing parenthesis and nothing triggered fallback to bad-url-token before that.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Feb 14, 2018

The Working Group just discussed [CSS2] empty url() behaviour needs errata to match Level 3, and agreed to the following resolutions:

  • RESOLVED: revert the change and open an issue noting that we had this change and we're not sure about it at this point.
The full IRC log of that discussion <dael> Topic: [CSS2] empty url() behaviour needs errata to match Level 3
<dael> github: https://github.com//issues/2211
<dael> fantasai: Basically gsnedders found css2 and css3 disagree what to do with an empty URL.
<dael> fantasai: Looks like we decided for L3 to make it invalid instead of refer to location of stylesheet. I couldn't find the minutes of that resolution so if anyone remembers that discussion it would be useful. We should make specs agree and impl to agree with specs.
<dael> astearns: Do we have data on current impl?
<dael> fantasai: gsnedders do you know? I tested using computed values wehre i got URL of the page, it was invalid
<dael> gsnedders: I think Chrome & FF resolve to page itself. I'm not sure.
<dael> astearns: Proposal is change L3 to match impl and revert the invalid resource bit?
<dael> fantasai: Would need to look what that looks like in the changeset. I have no real opinion on which way to go.
<dael> astearns: I spent time trying to find the resolution that lead to this and I failed as well.
<dael> astearns: What should we do fantasai ? Resolve to revert or do more research and see what's impl, have tests, and then decide on how to change L3?
<dael> fantasai: I thinkw ehave a fairly good idea. Test we ran so far it's not treated as invalid. It's supposed to be parsed as incorrect. Since this is a spec in CR and we have 2.1 and previous version say valid points at page we should revert. If someone thinks revist we can re-open it. That's my opinion because we want to update V&U.
<dael> fantasai: If we're not sure what to do don't make the change, file an issue, leave it open for later.
<dael> astearns: Given that we're still trying to figure out why we made the change it makes sense to open an issue.
<dael> fantasai: Then let's revert the change tot he draft and publish
<dael> astearns: What about revert change to draft, leave change and note as a comment in the draft so there's something in the source?
<dael> fantasai: I can mark it as an issue in the draft, this is CR spec. That text I can leave it commented in and that text is linked from the issue.
<dael> astearns: Sounds fair.
<dael> astearns: Proposal is revert hte change and open an issue noting that we had this change and we're not sure about it at this point.
<dael> astearns: Obj?
<dael> RESOLVED: revert the change and open an issue noting that we had this change and we're not sure about it at this point.
@tabatkins

This comment has been minimized.

Member

tabatkins commented Jun 21, 2018

Marking as Agenda+ to protest the resolution (I guess I wasn't able to make this call, seeing as I'm not in the minutes...). The reasoning for the change is stated clearly in the Note immediately following the quoted paragraph, which has been part of the draft since June 2016.

So I'd like to keep this change. The only observable difference is in how it's reflected in the OM; no place where url() is currently used can validly take a stylesheet, so treating url("") as a normal relative url (and thus pointing to the stylesheet itself) always results in a broken image/etc.

@gsnedders

This comment has been minimized.

Contributor

gsnedders commented Jun 21, 2018

so treating url("") as a normal relative url (and thus pointing to the stylesheet itself) always results in a broken image/etc.

That's surely not true with content negotiation? Or, heck, just crazy polyglot documents like http://incept10n.com/

@tabatkins

This comment has been minimized.

Member

tabatkins commented Jun 21, 2018

"always" should be read as "practically always"; insane polyglots or content-negotiation of this sort don't happen in practice and thus aren't relevant for us deciding what to do here.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Jul 2, 2018

The Working Group just discussed empty url behavior, and agreed to the following:

  • RESOLVED: Empty URLs are treated as "about:invalid"
The full IRC log of that discussion <fantasai> Topic: empty url behavior
<fantasai> github: https://github.com//issues/2211
<fantasai> ScribeNick: fantasai
<fantasai> TabAtkins: url() function with empty string as its argument...
<fantasai> TabAtkins: I changed spec ot match behavior of empty resource requests in HTML
<fantasai> TabAtkins: and y'all reverted the change because you didn't understand why I did it
<fantasai> TabAtkins: Answer is to be consistent with HTML!
<fantasai> TabAtkins: Empty URL is technically a relative URL to the resource itself
<fantasai> TabAtkins: But e.g. in HTML <img src=""> doesn't load the resource
<fantasai> TabAtkins: Empty URLs will still resolve in some cases, like in links
<fantasai> TabAtkins: But not for resource loading like this. Makes no sense to load the CSS stylesheet as an image of itself.
<fantasai> ...
<fantasai> chris: Do we expose imports in the OM, and would ...??
<fantasai> TabAtkins: I don't know
<fantasai> TabAtkins: But recursively attaching the same URL again wouldn't be useful
<fantasai> TabAtkins: I would like it to automatically fail, invalid URL.
<fantasai> tantek: But not invalid syntax
<fantasai> TabAtkins: No, not invalid syntax
<fantasai> TabAtkins: There's no place in CSS where resolving the URL as the URL of the stylesheet would be useful
<fantasai> TabAtkins: Proposal is that the empty URL, rather than being treated as a relative URL, is treated as an invalid URL
<fantasai> TabAtkins: Computed value is still an empty string, just treated like 'about:invalid'
<fantasai> TabAtkins: There should be no noticeable behavior difference, since the stylesheet is not a valid image/etc.
<tantek> +1
<fantasai> Rossen: Other comments?
<fantasai> RESOLVED: Empty URLs are treated as "about:invalid"
<TabAtkins> ScribeNick: TabAtkins
<TabAtkins> Rossen: So this is the last Values 3 issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment