Closed Bug 594505 Opened 10 years ago Closed 5 years ago

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


(Core :: ImageLib, defect)

Not set





(Reporter: dholbert, Unassigned)



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.
Summary: Paint broken-image icon for SVG images with parsing errors → Paint broken-image icon for <img src="foo.svg"> if foo.svg has parsing errors
  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.
Closed: 5 years ago
Resolution: --- → WORKSFORME
(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.)
Flags: needinfo?(dholbert)
[er, seth's already removing the comment -- hooray.]
Flags: needinfo?(dholbert)
^ 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.
You need to log in before you can comment on or make changes to this bug.