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

[css-images][css-masking][paint] Ambiguities in handling url() #383

Open
tabatkins opened this Issue Aug 4, 2016 · 17 comments

Comments

Projects
None yet
7 participants
@tabatkins
Member

tabatkins commented Aug 4, 2016

Back in 2012, roc raised this issue about an ambiguity in handling url()s that might refer to an image or an SVG element reference: https://lists.w3.org/Archives/Public/www-svg/2012Oct/0019.html In particular, giving a url like "mask-image: url(foo.svg#bar)", do we load that as an SVG image with a fragment of #bar, or do we load it as an SVG document and retrieve the id=bar element (which might be a <mask> element)? This also infects all CSS-<image>-taking properties, if we let CSS refer to SVG paint servers.

There was a lot of discussion and seemingly consensus, but nothing ever got formally decided, and several of the affected specs (Images, Masking, etc) weren't edited to take this into account. Let's fix that! The conclusion seemed to be:

  1. url()s without a fragment are always interpreted as images.
  2. url()s with a fragment vary slightly based on property: CSS-defined properties default to interpreting it as an image, while SVG-defined properties default to interpreting it as an element reference. You can force the image interpretation by instead loading it with image(), and the element interpretation by loading it as element().

This means that background-image can only refer to a paint server by using element("foo.svg#bar"), while mask-image can only use an SVG Stack by using image("foo.svg#bar").

It also means that in our new attempt to define fill and stroke for CSS, it'll default fragment urls to being element references, as that's their current behavior in SVG.

Let's please put this 4-year-old issue to rest - I can do the edits to the necessary specs.

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Aug 10, 2016

FYI:

I wrote up detailed URL-handling rules for SVG 2 as part of the overhaul of use-element handling. I tried to be consistent with CSS/FX specs (particularly Masking) to the extent that they had any guidance, but if you're adding new guidance you may want to review that section and make sure we're still consistent.

Key parts that are relevant to CSS:

  • If a property can only accept an image file reference, treat the URL as an image file. Any target fragment is used when rendering that image, but doesn't define a relevant document fragment.
  • If a property can accept either an image file or an element reference, and the referenced document can be parsed as a DOM, identify the targetted element to determine if it is a valid reference. If it is not, re-interpret the URL as an image file reference.
  • If a property can accept an element reference, and the URL does not have a target fragment, treat it as a reference to the root element when determining if it is a valid reference. This was based on a long-standing SVG feature request for <use> element references (to be able to use a separate SVG without editing it to add an id on the root element). If it is going to be a problem with CSS image properties, we may want to restrict that behavior.
  • External resources specified in style properties are fetched with CORS anonymous mode, so there shouldn't be security restrictions on whether the file is parsed as an SVG asset document or an SVG image.

I was only focusing on url() references, not on image() vs element(), and I was of course only focusing on SVG-defined properties.

I like the idea of using functions to clarify which way you want to interpret the reference. I'm slightly concerned about confusion with the other use of element(), to copy the rendered image data from an element in the current DOM. That's conceptually quite different from referencing an element that describes a graphical effect to apply.

I'm not too keen on having different rules for CSS-defined properties vs SVG-defined properties. That just seems like grounds for confusion. I realize there is a theoretical backwards-compatibility issue when allowing SVG paint servers as a CSS image type (a URL that would formerly be interpretted as an SVG image with :target styles now being interpretted as a paint server reference), but I think it is exceedingly rare in practice that :target styles are tied to a paint server element. The bigger concern would be whether there are substantial performance impacts from parsing the document to identify the element before deciding to treat it as an image: i.e., can implementations switch from "effects" mode to "image" mode without repeating the basic parsing & DOM construction?

@tabatkins

This comment has been minimized.

Member

tabatkins commented Aug 17, 2016

Hmm, so yeah, that's rather inconsistent with our earlier plans for CSS. :/ This is gonna need some discussion.

@astearns astearns removed the Agenda+ label Aug 24, 2016

@astearns

This comment has been minimized.

Member

astearns commented Aug 24, 2016

(removing label because this is already on the TPAC agenda)

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Aug 24, 2016

This is a big, long-term-strategy issue, so I appreciate that you're giving it extra thought. But FYI, we're aiming to get SVG 2 published as CR before TPAC, so requests to change anything normative will be a bit of a pain after that.

(Oh, and I won't be at TPAC myself, but do ping me if you have any questions about the SVG side.)

@tabatkins

This comment has been minimized.

Member

tabatkins commented Sep 19, 2016

After TPAC discussion:

  • I'm going to figure out precisely what properties are web-compat required to deal with this ambiguous syntax (where a url() can potentially be an image or a resource). I think it's just 'mask' and the 'marker-*' properties. (At least, that's the Chrome ones.)
  • For those required-ambiguous properties, we seem to want to go with the "fragment means resource, no-fragment means image" behavior; in particular, implementations do not want to load things twice.
  • We'll bind ourselves to not introduce ambiguity in the future; properties will be defined to have url() always be an image or ref, and you will have to switch behavior with image() or element(), as appropriate.

Extra detail: it looks like Chrome and Firefox do the split based on "local fragment" vs "otherwise". This means that external links, frag or no, are treated as image. Need more detail. /cc @birtles

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Sep 19, 2016

Re which properties currently have image-vs-element ambiguities:

Marker does not accept a plain image type, so there's no ambiguity there. Ambiguous URL properties I've noted that are currently supported in at least some browsers are mask and cursor; although we've deprecated the SVG <cursor> element, it's still supposed to be supported by user agents. The new SVG text layout also makes shape-inside and shape-subtract ambiguous.

In other words, mask should be the main focus for web compatibility issues. If no browsers currently support external file mask element references, that could be an argument for making special rules for same-document fragments. However, I think the more powerful argument is that mask-image is a new & still experimental property, that is not supported un-prefixed in any stable browsers, while mask with element references is well supported in SVG.

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Sep 19, 2016

By the way, one other detail I clarified in the SVG linking spec is that element references can be in any document that can be parsed as a DOM. I.e., you can link to an element in an HTML doc, not only an SVG doc. This is of course already supported in all the browsers for same-document references, but it was not defined for external references.

That shouldn't add too much confusion on this issue, but it means that in some cases there won't be a valid image interpretation of a file if the element reference interpretation fails.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Mar 15, 2017

Minutes from TPAC discussion on this issue: https://lists.w3.org/Archives/Public/www-style/2016Nov/0070.html

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Apr 19, 2017

The CSS Working Group just discussed Ambiguities in handling url(), and agreed to the following resolutions:

RESOLVED: mask-image distinguishes element reference vs image reference via local vs external reference in url()
The full IRC log of that discussion
<astearns> topic: Ambiguities in handling url()
<astearns> https://github.com/w3c/csswg-drafts/issues/383
<leaverou> https://github.com/w3c/csswg-drafts/issues/383
<astearns> github issue: https://github.com/w3c/csswg-drafts/issues/383
<fantasai> Github topic: https://github.com/w3c/csswg-drafts/issues/383
<leaverou> relevant TPAC minutes where this was discussed: https://lists.w3.org/Archives/Public/www-style/2016Nov/0070.html
<fantasai> TabAtkins: Everybody remembers issue from awhile ago -- it's ambiguous whether an URL with a fragid is referencing an element in the SVG or a pant server /mask whatever
<fantasai> TabAtkins: Everything is bad, and it's been bad since 2012 when issue was first raised by roc.
<fantasai> TabAtkins: Going forward we wanted to rely on element() vs image() functions to make it clear
<fantasai> TabAtkins: But still need to figure out what to do with url() legacy
<fantasai> TabAtkins: Unclear whether that should be property-specific, language-specific, something else
<fantasai> TabAtkins: Looking at TPAC minutes
<fantasai> TabAtkins: Some proposals
<fantasai> TabAtkins: 1. Treat ambiguous cases as url reference into an SVG document, not treat as image and apply :target
<fantasai> TabAtkins: 2. Treat ambiguous cases as url, if it has a fragID treat as a reference, otherwise treat as an image
<fantasai> TabAtkins: 3. Treat ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image
<fantasai> fantasai: 4. Do something different per property
<fantasai> plinss: #3 follows Web architectural principles better. Shouldn't judge URL by its syntax.
<fantasai> s/syntax/syntax like in #2/
<fantasai> Florian: #1 and #4 also don't violate principle
<fantasai> plinss: Could be a PNG at the end of a URL. You don't know
<fantasai> leaverou: Why would an author use a fragID on a PNG?
<fantasai> plinss: To crop a section of the PNG
<fantasai> plinss: We made image() function to make this unambiguous
<fantasai> TabAtkins: On the other hand, implementatiosn really don't want to load things twice
<fantasai> TabAtkins: Chrome and FF seem to decide on reference vs image based on whether it's local fragment vs external reference
<fantasai> TabAtkins: Could probably just switch on that, and then later introduce element() vs image() functions
<fantasai> TabAtkins: Might need more info from birtles
<fantasai> fantasai: I'm okay with distinguishing based on local vs external reference
<fantasai> TabAtkins: No browser currently allows external SVG references
<fantasai> dbaron: Gecko does in some cases, maybe not in CSS.
<fantasai> leaverou: Should not assume they never allow external SVG references
<fantasai> TabAtkins: Yeah, just don't need to consider it wrt web-compat
<fantasai> jet: We do for mask
<astearns> s/mask/clip-path/
<fantasai> dbaron: Most ambiguous case is mask
<fantasai> dbaron: But I thought we did for clip-path, filter, and mask
<fantasai> TabAtkins: mask is the only one that's troublesome atm
<fantasai> TabAtkins: Everyone else can define per property
<fantasai> TabAtkins: If a property only takes images, or only takes clip paths, not ambiguous
<fantasai> TabAtkins: Didn't want to do per-property decision for ambiguous cases
<fantasai> fantasai: Some of these properties that are currently unambiguous, maybe become ambiguous in the future
<fantasai> fantasai: So in the future, would be per-property disambiguation
<fantasai> TabAtkins: So when you said you handle external mask references, is that just for mask property that only accepts external references?
<fantasai> dbaron: We parse the mask property into longhands, so we would do it on mask-image, becaue that's where it lives
<fantasai> TabAtkins: How do you split mask-image into the two different cases?
<fantasai> TabAtkins: afaict, you did it based on whether local or not
<fantasai> fantasai: I think fill/stroke has (or will have) ambiguous cases.
<fantasai> ...
<fantasai> TabAtkins: Right now element() and image() hav other features, which is why were kicked out to L4
<fantasai> leaverou: element somewhat impl in Firefo
<fantasai> TabAtkins: We could though define them as subset of the functionality, i.e. same as url() except without ambiguity
<fantasai> fantasai: fill currently takes a paint server reference, we'lre adding image references, so it will become ambiguous
<fantasai> leaverou: Option 3 is off the table?
<fantasai> TabAtkins: Yeah, because cost-prohibitive
<fantasai> leaverou: #2 is only a problem cuz web architecture?
<fantasai> plinss: blatant violation of web architecture
<fantasai> fantasai: what about local ref vs external reff?
<fantasai> plinss: Not so bad
<fantasai> astearns: Sound sto me that 1 is only viable option
<fantasai> Florian: Well, 1 and 3?
<fantasai> s/3/4/
<fantasai> Florian: We will have to define property by property going forward
<fantasai> Florian: We do #1 on currently-ambiguous cases, but will have to define property by property going forward
<fantasai> TabAtkins: E.g. fill/stroke previously unambiguous, becoming mbiguous with new fill-stroke spec
<fantasai> astearns: So in cases where it coudl be interpreted as either, it will be ?
<fantasai> TabAtkins: Alternately could make it local vs external, like mask-image
<fantasai> fantasai: For mask-image, what are the possible things we could actually do?
<fantasai> TabAtkins: depends on what FF is doing and if compat-required
<fantasai> TabAtkins: I think local vs external is def how Webkit/Blink does it, and it's how Gecko used to work
<fantasai> birtles: Still
<fantasai> fantasai: So sounds like we *have* to do mask-image that way.
<fantasai> fantasai: Should we resolve on local vs non-local?
<fantasai> leaverou: Makes sense now because external reference aren't possible
<fantasai> fantasai: but we will have unambiguous syntax for that
<fantasai> leaverou: but by adding a fragID, author made their intent clear
<fantasai> TabAtkins: No, they didn't, because e.g. might be using svg stacks where fragID is triggerng :target
<fantasai> plinss: Can use a media frag to pull a frame out of an MP4
<fantasai> leaverou: ok...
<birtles> fwiw the Firefox code I'm looking at is http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/layout/style/nsCSSDataBlock.cpp#64 which at least indicates we don't trigger an image load if it's a local reference
<leaverou> s/leaverou: ok.../leaverou: ok/
<fantasai> dbaron and birtles investigate Gecko code
<fantasai> astearns: First proposed rsolution is distinguish these cases on mask-image via local vs external reference
<fantasai> astearns: which matches implementations, as far as we can tell
<fantasai> astearns: any objections?
<fantasai> dbaron: I'm not sure if that's actually happening. Things are fancy here.
<fantasai> dbaron: not sure if it matters
<fantasai> RESOLVED: mask-image distinguishes element reference vs image reference via local vs external reference in url()
<dbaron> The point where we actually branch between the SVG vs. image-tiling case is in the function PaintMaskSurface in nsSVGIntegrationUtils.cpp...
<dbaron> And as far as I can tell that's just a function of whether a pointer is null, which is null as a function of whether we managed to find an SVG Mask Element...
<fantasai> TabAtkins: I'm betting we could apply local vs externa globally, on account of local hash bg images are broken right now anyway, unless your HTML is also a PNG
<fantasai> leaverou: what about hashes on data URL? If SVG was in a data URL, and has a hash?
<fantasai> TabAtkins: That's still an external URL as far as loading pipeline is concerned
<fantasai> leaverou: I suspect that's something I've used...
<dbaron> the thing that it's testing for null was set up in the constructor nsSVGMaskProperty::nsSVGMaskProperty(
<fantasai> astearns: For second istance?
<fantasai> fantasai: second instance is now, fill-stroke
<fantasai> astearns: Maybe wait until we have more info?
<fantasai> astearns: e.g. dbaron figuring out gecko code
<fantasai> xidorn: Person who implemented mask-image says we do check if target element is mask element after we load the file
<fantasai> fantasai: Question is, afte ryou check it and it fails the check, what do you do?
<fantasai> dbaron: I suspect we do two loads
<fantasai> plinss: Don't want to require impls to all do hardest least performant thing, but don't want to preclude doing it correctly
<fantasai> astearns: Seems we have t leave this one for now
<fantasai> dbaron: Anyone have a technique for making a file that's both a valid SVG and a valid PNG?
<fantasai> Florian: I don't but I know who would
<fantasai> dbaron: Might be able to test SVG as SVG
<fantasai> Florian: If you need, try p01 on twitter, he makes crazy demos
<fantasai> dbaron: I think we just need an SVG that's a circle and a mask that's a square
@tabatkins

This comment has been minimized.

Member

tabatkins commented Apr 19, 2017

We're also gonna go ahead and use local/external as the determiner for the now-ambiguous fill/stroke properties.

And we might be able to do it globally:

  • 'background-image' refs that have "url(#foo)" are broken today, I think, and so will continue to be broken even if we treat them as (local) element references. (Today they refer to the stylesheet's URL, with the hash setting the :target pseudo if it's HTML. The result is then loaded as an image, so it only does something useful in the silly polyglot-html-and-png tricks.)
  • <use> doesn't currently, in many (all?) browsers, allow external references (despite SVG defining them as allowed), so it should be okay for them to interpreted as an image ref, and thus being invalid and ignored.
@fantasai

This comment has been minimized.

Contributor

fantasai commented Apr 19, 2017

We didn't actually resolve on anything except mask-image.

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Apr 19, 2017

Wait, does this mean that you won't be able to use SVG mask elements in an external file? That is tragic. I'm pretty sure it's supported in at least Firefox, maybe Edge.

<use> in external files is supported in all modern browser, provided it is same-origin. (Cross-origin references aren't supported anywhere, including in the spec.)

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Apr 19, 2017

And for fill & stroke, again references to another file are currently only supported in Firefox, old Presto, and Edge under certain possibly accidental conditions. But I was really hoping that a consistent model with CSS URL references would lead to browsers fixing that. It seriously limits the ability to create re-usable assets for inline SVG icons if you need to copy & paste (or PHP include, or whatever) your gradients into every page of the website.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Apr 21, 2017

The CSS Working Group just discussed Update on URL function discussions, and agreed to the following resolutions:

  • RESOLVED: Should treat ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image
  • RESOLVED: Implementations should treat �1ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image�
  • RESOLVED: For ambiguous cases, UAs SHOULD first see if there's an appropriate reference, otherwise go back and reload as an image
The full IRC log of that discussion
<Rossen> Topic: Update on URL function discussions
<myles> plinss: url() - do they resolve to images or elements?
<myles> plinss: we had 4 options
<dbaron> https://dbaron.org/css/test/2017/mask-url
<myles> plinss: David looks into what Firefox does. But our resolution is contrary to that. We want to revisit to allow Firefox
<myles> dbaron: Firefox treats mask image with a URL. If it has a hash, we will look for a ref in it, whether its local or remote
<plinss> present+
<myles> dbaron: so, there are some cases where we will try to load it twice. If there is no hash, we don't attempt to look for a mask element, and load it as an image. If it's purely a local ref, we don't' attempt to load it as an image - we will only find a mask URL. But if it has a hash in the middle, we'll load it both ways.
<myles> fantasai: This is an optimization of loading twice in general
<myles> fantasai: a local reference is not going to be an image
<myles> dino: why not?
<myles> dino: it could be an SVG image, or a SVG root, or a canvas
<myles> dbaron: we dont' want infinite recursion
<myles> TabAtkins: pointing to canvases isn't allowed at all anywhere
<myles> dino: Someone might do it
<myles> dino: We could point as an image element in the document and using it as a mask
<myles> leaverou: that's not allowed
<myles> ChrisL: this is imaginary
<myles> dbaron: the mask property used to point to a mask element but we extended it to point to image
<myles> dbaron: when we do this, we ignore the other masking properties like mask-repeat, etc.
<myles> ChrisL: this is according to the spec, rights?
<myles> dbaron: i think so
<myles> leaverou: So, it's possible to have the Firefox implementation from the spec?
<myles> leaverou: We all agree that this is the most reasonable choice
<myles> Rossen: Can someone summarize what is going on?
<myles> plinss: You should know what's going on
<myles> leaverou: Hey tab, please type in the options again
<plinss> https://log.csswg.org/irc.w3.org/css/2017-04-19/#e796988
<leaverou> 1. Treat ambiguous cases as url reference into an SVG document, not treat as image and apply :target
<leaverou> 2. Treat ambiguous cases as url, if it has a fragID treat as a reference, otherwise treat as an image
<leaverou> 3. Treat ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image
<leaverou> 4. Do something different per property
<myles> leaverou: ❤️
<myles> dino: what does "ambiguous" mean?
<myles> plinss: if the property can handle both an element and an image, and the URL could be either
<myles> plinss: can we resolve to make this decision on whether it's a local or external URL?
<myles> myles: Firefox does 3, right?
<myles> leaverou: Basically. With some extra optimizations
<myles> plinss: Architecture says that you can only interpret what a fragment means until you get a content-type from the response
<myles> dbaron: The case where Firefox doesn't do the right thing is when there is a base URL and a url(#foo)
<myles> dbaron: This is probably not the only case where this occurs
<myles> dbaron: like <a href="#foo"> scrolls within the current document
<myles> TabAtkins: CSS always treats a #url as a reference into the local document
<myles> plinss: That is a separate issue and an architectural violation
<myles> plinss: At the end of the day, I'm okay with browsers not handling everything exactly b/c of optimizations. What I'm not okay with is the spec stating you must violate architectural principles and Gecko must stop doing what it's doing
<myles> Rossen: Other thoughts?
<myles> Rossen: Proposed resolution: a mixture of #2 and #3
<myles> fantasai: By choosing to follow Moz behavior, you are deciding tha ta local reference will always be an element reference and not an image reference. Which is fine, but we should be explicit.
<myles> fantasai: We always have our image() and element() functions which can make the distinction
<myles> ChrisL: so you're not losing anything
<myles> TabAtkins: We know the content type of the current resource, so we can interpret the hash correctly
<myles> TabAtkins: I need confirmation from our loading people that this is implementable
<myles> TabAtkins: we will try
<myles> plinss: I'm okay with "should" or "must (but we know you won't)"
<myles> Rossen: "should" please.
<myles> Rossen: Proposed resolution: Option #3 with "should"
<myles> TabAtkins: 👍
<myles> TabAtkins: (summarizes proposed resolution)
<leaverou> RESOLVED: Should treat ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image
<myles> RESOLVED: Implementations should treat �1ambiguous cases, load it twice -- first see if there's an appropriate reference, otherwise go back and reload as an image�
<dbaron> Github issue: https://github.com/w3c/csswg-drafts/issues/383
<leaverou> RESOLVED: For ambiguous cases, UAs SHOULD first see if there's an appropriate reference, otherwise go back and reload as an image
<myles> Thanks leaverou
<myles> Rossen: RESOLVED: Let's stop resolving
@astearns

This comment has been minimized.

Member

astearns commented Apr 21, 2017

The last resolution is the actual one

@AmeliaBR

This comment has been minimized.

AmeliaBR commented Apr 21, 2017

The last resolution is the actual one

That would be "RESOLVED: Let's stop resolving" ?


For ambiguous cases, UAs SHOULD first see if there's an appropriate reference, otherwise go back and reload as an image

So, does this mean that my URL-resolution algorithm from SVG 2 is still valid?

Although, now I read it again I realize that I should have been more clear that there are two cases for what to do if a target element can't be located, depending on whether it is a same-file vs different file references.

  • Same file: if the target element doesn't exist or is the wrong type, listen for DOM mutations in case the problem is fixed later.
  • Different file: if the target element doesn't exist or is the wrong type, and property can also accept an image, then re-process the referenced file as an image resource.
@rocallahan

This comment has been minimized.

rocallahan commented Mar 28, 2018

I thought I mentioned this on www-style a few years ago but I can't find it now: I had come to the conclusion that the way to resolve this issue was to unify the image-loading path with the element-reference path, basically by eliminating the latter. You would use the image-loading path, and if you happened to load an SVG image and the fragment reference points to the right kind of element, you would use that instead of rendering the image. (Document self-references would continue to be special-cased.) This depends on the browser being able to "punch through" the image-loading-and-rendering abstraction to access the underlying image's DOM, but I guess that's not a problem in practice.

This would mean that "external resource documents" are subjected to the constraints of SVG image documents, e.g. they can't contain external references of their own. That seems like an insignificant loss in practice, unless Web-compat issues have developed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment