737778 - SVGDocumentWrapper should not touch nsIParser

Status: NEW

(Core :: SVG, defect)

Description

[Henri Sivonen (:hsivonen)]

SVGDocumentWrapper assumes that you can QI a stream listener to nsIParser and further assumes that it's OK to force-feed data to the parser from OnStopRequest by repeatedly calling ContinueInterruptedParsing.

Neither of these assumptions is planned to hold when bug 363688 is fixed. Thus, this blocking bug 363688.

My understanding is that SVGDocumentWrapper is this way, because image handling code assumes that the entire image is available after OnStopRequest has returned. We need to get rid of that assumption.

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Henri Sivonen (:hsivonen) from comment #0)
> My understanding is that SVGDocumentWrapper is this way, because image
> handling code assumes that the entire image is available after OnStopRequest
> has returned.

Technically, imagelib assumes that the image's *size* (width & height) are available when OnStopRequest has returned, IIRC.

For SVG, that assumption effectively ends up being the same as assuming "the entire image is available", because for all we know, the width & height could be at the end of the document, e.g. in a situation like this...
>  <!-- 10 megabytes of comments / XML entities / etc  ... -->
>  <svg xmlns=... width="100px" height="100px"></svg>
..in which case we have to get the parser virtually all the way to the end of the image before we find out the width/height.

> We need to get rid of that assumption.

Yeah. :-/  I haven't been following imagelib closely for a little while... joe/bholley, do you know if any progress has been made on that rearchitecting?

This probably belongs in Core | Imagelib, since the rearchitecting is the bulk of the work that needs to be done for this. Alternately, we could to file a bug on "don't assume height/width are available when OnStopRequest completes" in Imagelib and have this depend on it. (joe/bholley -- do you know if a bug already exists on that?)

Comment 2

[Bobby Holley (:bholley)]

The rearchitecting is bug 505385. I've been too busy in the last few months with DOM stuff to touch it, though. Hopefully that'll change soonish.

Maybe make a comment there that this assumption needs to be revisited? I won't promise to do that (I'm trying to keep it mostly a refactor, and avoid changing too much behavior), but it should be much easier to fix after I'm done.