594505 - Paint broken-image icon for <img src="foo.svg"> if foo.svg has parsing errors

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[Daniel Holbert [:dholbert]]

If we have an <img> element targeting an SVG file that has parsing errors (e.g. no SVG root node), I think we'd like to paint the broken-image icon, to offer web authors feedback about what's going wrong.  Currently (with bug 276431's patches), we paint the "object" icon instead.

Here's the chunk of code involved:
> > VectorImage::OnStopRequest(nsIRequest* aRequest, nsISupports* aCtxt,
> >                            nsresult aStatus)
> 
> >+  nsresult rv = mSVGDocumentWrapper->OnStopRequest(aRequest, aCtxt, aStatus);
> >+  if (!mSVGDocumentWrapper->ParsedSuccessfully()) {
> >+    // XXXdholbert Need to do something more here -- right now, this just
> >+    // makes us draw the "object" icon, rather than the (jagged) "broken image"
> >+    // icon.
> >+    mError = PR_TRUE;
> >+    return rv;
> >+  }

I chatted with bholley and joe in IRC - the upshot was that OnStopDecode might be what I want to call (with an "error" status argument), but it's tricky.

The thing is, imagelib currently expects us to fire OnStopDecode *right before* we signal that we've finished receiving all the data.  We could satisfy that requirement for now, but we might not want to have that bind our hands in the future -- i.e. we might want to do the SVG parsing asynchronously, after we've finished receiving data, and at that point (when we discover parsing errors) it'd be too late to call OnStopDecode.

Comment 1

[Daniel Holbert [:dholbert]]

Testcase:
  data:text/html,<img src="data:image/svg+xml,<">

This renders a broken-image icon now (at least in Nightly) (thanks to Seth for noticing), so I think we can call this WORKSFORME.

Comment 2

[Daniel Holbert [:dholbert]]

(The code-comment in comment 0 probably needs updating or removing, because it's no longer correct about the outcome and because it references this bug number. Setting ni=me to do that.)

Comment 3

[Daniel Holbert [:dholbert]]

[er, seth's already removing the comment -- hooray.]

Comment 4

[Seth Fowler [:seth] [:s2h]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/51242f7af759

Comment 5

[Seth Fowler [:seth] [:s2h]]

^ Comment removed.

FWIW, the root cause of this was probably that we were firing SIZE_AVAILABLE in the parse error case. At some point we stopped doing that.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/51242f7af759