Filing this bug on applying s/GetDisplayDocument()/IsResourceDoc()/ to the early return in nsFrameLoader::MaybeCreateDocShell, so that check will catch SVG-as-an-image.
(Right now we inadvertantly catch SVG-as-an-image through an IsActive() check, but we should check more clearly/explicitly.)
See bug 596765 comment 38 for details.
Created attachment 573064 [details] [diff] [review]
This patch switches to use IsResourceDoc() as described above.
It also tweaks the existing reftests img-foreignObject-embed-* as follows:
- File-rename -- s/embed/iframe/ -- for test w/ iframe, for clarity
- I modified the SVG helper-file for the test formerly known as img-foreignObject-embed-1.html, such that its <div> is now the *parent* instead of the *sibling* of the <iframe>, so it'll no longer push the <iframe> out of view. So now, we'd see the <iframe> in the image, if it were being honored.
- I also split off another version of that test (-1b.html / -1b-helper.svg) which has no alert() and *just* tests for <iframe> rendering (for the purposes of this next part):
- I added "!=" lines to ensure that each of the SVG resource files don't match lime.svg when viewed directly. (though I couldn't do that for the one that uses alert(), since it'd hang the reftest process if viewed directly)
Created attachment 573066 [details] [diff] [review]
patch 1 v1
(same patch, just corrected a whitespace issue & alphabetical-ordering in the tweaked reftest.list chunk)
Created attachment 573067 [details] [diff] [review]
patch 2 v1 (just a file rename)
This patch just renames img-foreignObject-embed-2* to img-foreignObject-embed-1*, since the old img-foreignObject-embed-1* files are now moved out of the way (since they really use iframe instead of embed).
(I probably could have done this as part of patch 1, but I don't trust hg to do the right thing if I "hg mv XX YY; hg mv ZZ XX" all in the same cset. (and it feels cleaner this way))
Created attachment 574118 [details] [diff] [review]
patch 3 v1: s/GetDisplayDocument/IsResourceDoc/ in ShouldLoad
We should make the same sort of change (s/GetDisplayDocument/IsResourceDoc/) in nsDataDocumentContentPolicy::ShouldLoad, too. Right now ShouldLoad has an early-return for non-external-resource-documents (and then places additional restrictions on external resource documents), and this patch tightens that early-return so that we won't take it for image documents.
This is actually the change that bz originally suggested tweaking back in bug 596765 comment 1. I hadn't made this change yet, though, because we never actually reach this code for e.g. <iframe> in SVG-as-an-image -- we reject that earlier, in nsFrameLoader::MaybeCreateDocShell (in the chunk of code touched by patch 1 here). However, if we bypassed that earlier check for some reason, we'd definitely want ShouldLoad to reject it.
This change also will make ShouldLoad clearer -- there's no reason the already-existing check should apply *just* to external resource documents but not to SVG-as-an-image.
Created attachment 574119 [details] [diff] [review]
patch 3 v2: s/GetDisplayDocument/IsResourceDoc/ in ShouldLoad
(just fixed a comment after the code-change, to reflect the new behavior)
Landed patch 3: