746632 - If no fallback colour is specified we shouldn't draw anything when the URL fails to resolve

Status: RESOLVED FIXED

(Core :: SVG, defect)

Comment 1

[Robert Longson [:longsonr]]

Per the 2nd edition SVG testsuite test in the URL.

Comment 2

[Robert Longson [:longsonr]]

Attachment: patch

Comment 3

[Robert Longson [:longsonr]]

The specification text in question is here: http://www.w3.org/TR/SVG/painting.html#SpecifyingPaint 

In this case the document is in error so we shouldn't render that element.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 616182 [details] [diff] [review]
patch

This seems entirely reasonable, but I really don't see anything in the spec that says this.  All I see in the spec is:

      If the IRI reference is not valid (e.g., it points to an object that doesn't exist or the object is not a valid paint server), then the paint method following the <funciri> (i.e., none | currentColor | <color> [<icccolor>] is used if provided; otherwise, the document is in error (see Error processing).

which implies to me that the spec says the document is in error in this case.  (I'm looking at REC-SVG11-20110816, which I have an offline copy of; I'm on a slow network right now and the URL you gave doesn't load).

So r=dbaron, I suppose, since the behavior seems better even though the spec doesn't (AFAICT) say that either old or new behavior is correct.

Seems like it's worth raising a spec or testsuite issue unless I'm missing something in the spec, though.  (Does this change us to match other browsers?)

Comment 5

[Robert Longson [:longsonr]]

(In reply to David Baron [:dbaron] from comment #4)
> 
> which implies to me that the spec says the document is in error in this
> case.  (I'm looking at REC-SVG11-20110816, which I have an offline copy of;
> I'm on a slow network right now and the URL you gave doesn't load).

Right.

> 
> So r=dbaron, I suppose, since the behavior seems better even though the spec
> doesn't (AFAICT) say that either old or new behavior is correct.
> 
> Seems like it's worth raising a spec or testsuite issue unless I'm missing
> something in the spec, though.  (Does this change us to match other
> browsers?)

There is a testsuite test for it - the URL in this bug points to it.

We would match Opera and IE9 with this fix.

Comment 6

[Robert Longson [:longsonr]]

pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/956287d79825

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/956287d79825