when moving the mouse over an "iframe" in print preview, the content of it disappears after returning to the page. this seems to be a regression from Bug 284664

an example of the problem

In print preview, if I move the mouse outside the iframe I get a continuous stream of: ###!!! ASSERTION: Mouse move must have some target content: 'targetElement', file /home/bzbarsky/mozilla/xlib/mozilla/content/events/src/nsEventStateManager.cpp, line 2682 And when I open up print preview once the iframe has gone blank, I get: ###!!! ASSERTION: null window: 'mWindow', file /home/bzbarsky/mozilla/xlib/mozilla/layout/base/nsDocumentViewer.cpp, line 1671

The assertions about no target for the mouse move are fairly straightforward ... there is no content corresponding to the page frames and the viewport area outside the pages, so no target content for mouse moves over those frames. This should be fairly innocuous. I can patch that up so that GenerateMouseEnterExit treats the mouse as over the root element if it can't find any other target content. But how this manages to nuke the IFRAME at end of print preview, I'm not sure. I do get this assertion: ###!!! ASSERTION: no document: 'doc', file ../../../../../content/xul/content/src/nsXULElement.cpp, line 2376 Break: at file ../../../../../content/xul/content/src/nsXULElement.cpp, line 2376 On the stack, some script is trying to execute a "cmd_undo".

This is a better testcase: https://bugzilla.mozilla.org/attachment.cgi?id=179686 I've noticed that the IFRAME contents only get thrown away if we move the mouse out of it during print preview.

The problem is that the ESM code to propagate mouse events down into subdocuments was calling nsSubdocumentFrame::GetDocShell. This has the side effect of loading the subdocument if it hasn't already started loading. In particular it hooks up nsSubdocumentFrame::mFrameLoader to the frame loader. Then nsSubdocumentFrame::Destroy thinks it's responsible for telling mFrameLoader's document viewer to Hide() ... even though this document viewer is the docviewer for the main presentation, and we're in the print preview presentation. That would be OK because Hide() checks to see if it's in Print or PrintPreview and ignores the hide request if so. Unfortunately the comments lie and the check always returns PR_FALSE in subdocuments, because they don't have a pointer to the print engine. So we go ahead and tear down the subdocument's main presentation. The approach I'm taking here is to simply not call GetDocShell with that chain of nasty side effects ... instead we can get the subdocument's prescontext/ESM by walking the view tree. This doesn't work in print/print preview because they don't have the view trees hooked up, but who cares. This patch also includes a fix to stop those assertions firing. And it documents the lies in DocumentViewerImpl.

Comment on attachment 179821 [details] [diff] [review] fix I guess this is ok as wallpaper, but could you file a bug on fixing the nsDocumentViewer code? It should probably call internal methods that check its parents too, not the frozen api functions it's calling now. If you don't have time to fix that, let me know and I'll look for someone to do it, or do it myself. Once that's done, do we want to go back to using GetDocShell, or leave things as they are?

Comment on attachment 179821 [details] [diff] [review] fix fairly straightforward fix to a major regression

There are really a couple of bugs here: 1) DocumentViewerImpl doesn't ignore Hide() in print/print preview for subdocuments. 2) nsSubDocumentFrame::GetDocShell shouldn't have the side effect of triggering document load. It should be called ObtainDocShell or something like that, and if necessary, a non-side-effecting GetDocShell should be added. In particular nsSubDocumentFrame shouldn't be deciding whether to call Hide() based on whether GetDocShell was ever called or not. Honestly, I don't fully understand the lifetime issues and relationships between the docshell, the documentviewer, the frameloader, and how the print engine massages them. But you're right that this fix isn't great. Adding new API as a band-aid is not a good idea. I'll submit another patch which is less band-aid-y and at least doesn't add new API.

This approach just makes the subdocument frame not call Hide() if it didn't create the presentation in ShowDocShell. That seems reasonable. It certainly fixes the bug and doesn't require any new interface.

> Honestly, I don't fully understand the lifetime issues and relationships > between the docshell, the documentviewer, the frameloader, and how the print > engine massages them. That's because the relationships are crappy. The document viewer is sort of a per-presentation object, except there's one particular one that the docshell holds a reference to... The rest of the mess is corollaries of this. :(

Comment on attachment 179891 [details] [diff] [review] fix #2 Yeah, this is better. Maybe s/ObtainDocShell/EnsureDocShell/ or something?

Comment on attachment 179891 [details] [diff] [review] fix #2 okay, this is the preferred fix for this unfortunate regression.

Comment on attachment 179891 [details] [diff] [review] fix #2 a=asa for 1.8b2 checkin.

checked in

is this bug somehow related to Bug 176611 ?

Probably not.

Note: this caused bug 289719