473962 - Can't print Messages, App process hangs after closing

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tobias Fischer]

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090116 Mnenhy/0.7.5.20005 SeaMonkey/2.0a3pre

Printing Messages from MailNews or Thunderbird does not work anymore. This regressed between:
2009011304 (last known good SM Nightly) and
2009011400 (first tested not working SM Build)

Also tested with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090116 Shredder/3.0b2pre

When try to print an Message via clicking O.K. in Printer Dialog, the normally upcomming process-popup does not come up. In Statusbar the Text "Printing Message..." shows up, but the Message was not printed, and there are no data sent to the printer. 

When I try to close the Application, the process is still running and must be killed via Task Manager. 

No Errors in Error-Console. 

Even when printing Messages is not the most used thing, I think this might block 1.9.1.

Comment 1

[Karsten Düsterloh]

Confirming both no print and hang with my SM debug c-c build from yesterday under Linux, hence OS=ALL.

Comment 2

[Wayne Mery (:wsmwk)]

I don't even get that - no output or print window is generated.  but also no hang here.  Print preview works.  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090116 Shredder/3.0b2pre

how is this a blocker? certainly dogfood. no doubt blocking 1.9.1. but blocking development and testing?

Comment 3

[Frank Wein [:mcsmurf]]

Checkins in timeframe comm-central:
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2009-01-13+03%3A30+&enddate=2009-01-14+01%3A00%3A00
Checkins in timeframe m-c 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=2009-01-13+03%3A30+&enddate=2009-01-14+01%3A00%3A00

Comment 4

[Frank Wein [:mcsmurf]]

This has been caused by Bug 465993, tested with hg bisect and manually verifying this with local backout and checkin of that changeset.
Probably no blocker, but at least critical (though this bug does not really fit the criteria for critical).

Comment 5

[Frank Wein [:mcsmurf]]

As a warning: If someone wants to test/fix this, note that SeaMonkey currently crashes on startup due to Bug 473933. So either use Thunderbird to test this or launch SeaMonkey with seamonkey.exe -mail

Comment 6

[Boris Zbarsky [:bzbarsky]]

Did this use to work before the checkin for bug 112294?

I'll try once again to get seamonkey building, but I haven't had much luck with it the last few times I tried...

Comment 7

[Frank Wein [:mcsmurf]]

Bug 112294 and Bug 465993 backed out locally: works
Bug 112294 backed out locally and Bug 465993 not: does not work.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ok.  Bug 465993 was just moving around and modifying slightly the check that bug 112294 added.  I'm surprised you managed to back out bug 112294 without backing out bug 465993.

In any case, I'll take a closer look on Monday.

Comment 9

[Frank Wein [:mcsmurf]]

Not backing out Bug 465993 required a bit of manual work, yes.

Comment 10

[Boris Zbarsky [:bzbarsky]]

OK... So I don't get a dialog as described in comment 0 at all when printing to file on Mac, nor any attempt to put one up.

Frank, would you be able to set a breakpoint at the error return bug 465993 added and see whether it gets hit (I assume it does).  If it does, can you post the stack?

Comment 11

[Frank Wein [:mcsmurf]]

Yes, it does get hit. Stack:
embedcomponents.dll!nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow * aParent=0x138b8d68, const char * aUrl=0x06a8f0c0, const char * aName=0x0012f894, const char * aFeatures=0x0012f840, int aDialog=1, nsIArray * argv=0x072a77e8, int aCalledFromJS=0, nsIDOMWindow * * _retval=0x0012f81c)  Zeile 662
embedcomponents.dll!nsWindowWatcher::OpenWindow(nsIDOMWindow * aParent=0x138b8d68, const char * aUrl=0x06a8f0c0, const char * aName=0x0012f894, const char * aFeatures=0x0012f840, nsISupports * aArguments=0x072a77e8, nsIDOMWindow * * _retval=0x0012f81c)  Zeile 421 + 0x24 Bytes
gklayout.dll!nsGlobalWindow::OpenInternal(const nsAString_internal & aUrl={...}, const nsAString_internal & aName={...}, const nsAString_internal & aOptions={...}, int aDialog=1, int aContentModal=0, int aCalledNoScript=1, int aDoJSFixups=0, nsIArray * argv=0x00000000, nsISupports * aExtraArgument=0x17f024a8, nsIPrincipal * aCalleePrincipal=0x0b8d39e0, JSContext * aJSCallerContext=0x00000000, nsIDOMWindow * * aReturn=0x0012f9fc)  Zeile 7196 + 0x4c Bytes
gklayout.dll!nsGlobalWindow::OpenDialog(const nsAString_internal & aUrl={...}, const nsAString_internal & aName={...}, const nsAString_internal & aOptions={...}, nsISupports * aExtraArgument=0x17f024a8, nsIDOMWindow * * _retval=0x0012f9fc)  Zeile 5003 + 0x2e Bytes
embedcomponents.dll!nsPrintProgress::OpenProgressDialog(nsIDOMWindowInternal * parent=0x00000000, const char * dialogURL=0x011a6dec, nsISupports * parameters=0x0012f9fc, nsIObserver * openDialogObserver=0x0a59e428, int * notifyOnOpen=0x17f024a8)  Zeile 142 + 0x54 Bytes
embedcomponents.dll!nsPrintingPromptService::ShowProgress(nsIDOMWindow * parent=0x138b8d68, nsIWebBrowserPrint * webBrowserPrint=0x0a07906c, nsIPrintSettings * printSettings=0x0aba08a8, nsIObserver * openDialogObserver=0x0a49a868, int isForPrinting=1, nsIWebProgressListener * * webProgressListener=0x0012fa80, nsIPrintProgressParams * * printProgressParams=0x13c6c218, int * notifyOnOpen=0x0012fab8)  Zeile 247
gklayout.dll!nsPrintEngine::ShowPrintProgress(int aIsForPrinting=1, int & aDoNotify=1)  Zeile 1016
gklayout.dll!nsPrintEngine::DoCommonPrint(int aIsPrintPreview=327067168, nsIPrintSettings * aPrintSettings=0x0a3ce400, nsIWebProgressListener * aWebProgressListener=0x17974a48)  Zeile 708
gklayout.dll!nsPrintEngine::CommonPrint(int aIsPrintPreview=0, nsIPrintSettings * aPrintSettings=0x0aba08a8, nsIWebProgressListener * aWebProgressListener=0x06e5f07c)  Zeile 418 + 0x10 Bytes
gklayout.dll!nsPrintEngine::Print(nsIPrintSettings * aPrintSettings=0x0aba08a8, nsIWebProgressListener * aWebProgressListener=0x06e5f07c)  Zeile 725
gklayout.dll!DocumentViewerImpl::Print(nsIPrintSettings * aPrintSettings=0x0aba08a8, nsIWebProgressListener * aWebProgressListener=0x06e5f07c)  Zeile 3583 + 0xe Bytes
gklayout.dll!DocumentViewerImpl::PrintWithParent(nsIDOMWindowInternal * aParentWin=0x0cb30018, nsIPrintSettings * aThePrintSettings=0x0aba08a8, nsIWebProgressListener * aWPListener=0x06e5f07c)  Zeile 2624
mail.dll!nsMsgPrintEngine::PrintMsgWindow()  Zeile 662
mail.dll!nsPrintMsgWindowEvent::Run()  Zeile 713
xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fc78)  Zeile 511
xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=1)  Zeile 227 + 0xd Bytes
gkwidget.dll!nsBaseAppShell::Run()  Zeile 170 + 0x9 Bytes
tkitcmps.dll!nsAppStartup::Run()  Zeile 193
xul.dll!XRE_main(int argc=1, char * * argv=0x00a56ff8, const nsXREAppData * aAppData=0x00a57490)  Zeile 3283
seamonkey.exe!NS_internal_main(int argc=1, char * * argv=0x00a56ff8)  Zeile 104
seamonkey.exe!wmain(int argc=10842104, unsigned short * * argv=0x00a56f70)  Zeile 89
seamonkey.exe!__tmainCRTStartup()  Zeile 583 + 0x17 Bytes

Comment 12

[Boris Zbarsky [:bzbarsky]]

Hmm.  So the window passed as the parent into nsWindowWatcher::OpenWindowJSInternal is the window of the document we're printing, looks like, which is indeed not chrome.  But that document is visible, isn't it?  Ere, what's up here?

Moving to the right component so that we can get this triaged fro 1.9.1.

Comment 13

[Frank Wein [:mcsmurf]]

BTW: When you open Print Preview for that mail and then click on the "Print" button in that preview window, printing works.

Comment 15

[Ere Maijala (slow)]

Taking.

Comment 16

[Ere Maijala (slow)]

Attachment: Patch v1

This patch fixes the problem. We don't really need to guard against hidden parent unless the window is modal, because there is no danger of ending in a modal loop that cannot be broken.

It's still strange that the document has a hidden parent.

Comment 17

[Boris Zbarsky [:bzbarsky]]

If that's what we want to check, just check for the CHROME_MODAL flag, instead of CHROME_DEPENDENT.

But that's just a dodge, since if this code happened to be opening a modal dialog (instead of the non-modal dependent one it actually opens) it sounds like we'd still lose.  So the real question is why something is coming up invisible here.  given the steps to reproduce it doesn't sound like there should be any invisible windows in sight.

Comment 18

[Ere Maijala (slow)]

Ok.

The invisible window is there and it will stay there even after printing is done. I don't know what it really is, but I'll try to find out.

Comment 19

[Ere Maijala (slow)]

The invisible window seems to be nsDocument's mScriptGlobalObject. Apparently we stumble because mScriptGlobalObject is not chrome. I don't know enough about the mechanism to say if/how to change it.

Comment 20

[Ere Maijala (slow)]

Attachment: Patch v1.1 [Checkin: Comment 32 & 37]

Practically same but more elegant patch.

Comment 21

[Boris Zbarsky [:bzbarsky]]

> The invisible window seems to be nsDocument's mScriptGlobalObject.

Hold on.  Any nsGlobalWindow is the mScriptGlobalObject of the document in it, basically.  The window watcher code does the following:

1) Start with an nsIDOMWindow (in this case presumably a content window, since
   we hit this condition).
2) Get the corresponding treeowner.  So this would be an nsContentTreeOwner.
3) Get the nsIBaseWindow interface from the treeowner.  This basically QIs to
   nsIBaseWindow.
4) Get the main widget from the basewindow.  In this case, that hands back the
   nsXULWindow's mWindow.
5) Do  a visibility check on the widget.  No idea what this does on Windows.

So the question is what URI is loaded in the relevant nsXULWindow's mDocshell?  Why is its mWindow invisible, exactly?

Comment 22

[Boris Zbarsky [:bzbarsky]]

OK.  Frank sat down and stepped through this, which was a great help.  Here's the deal.  When it goes to print, mailnews opens msgPrintEngine.xul, which is a XUL window with a content display area.  It loads the message in this content display area.  The window loads msgPrintEngine.js and calls InitPrintEngineWindow.

This function calls nsMsgPringEngine::ShowWindow(PR_FALSE), which goes and hides the XUL window.  It's still open, but it's now hidden.  It'll get reshown if someone does print preview.  Basically, we create the preview, but hide it unless someone asks for it.

Then we go to print, and the nsPrintEngine uses the window of the document being printed (which is NOT a chrome window) as its parent window for the progress dialog.  So that fails, of course...

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 358587 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 32 & 37]

I guess we have to do this, and file a followup bug on mailnews to fix their stuff to not open up windows from hidden windows... :(

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 358587 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 32 & 37]

Approving this to land. I don't know that I can argue that this is a blocker for 1.9.1, but given the patch I'd be happy to take it for 1.9.1

Comment 25

[David Ascher (:davida)]

This is pretty important for tb3, obviously.

Comment 26

[Tobias Fischer]

(In reply to comment #24)
> (From update of attachment 358587 [details] [diff] [review])
> Approving this to land. I don't know that I can argue that this is a blocker
> for 1.9.1, but given the patch I'd be happy to take it for 1.9.1

Sorry, but I was ill the last week, so I have missed to cancel the blocking request in time.

Comment 29

[Mark Banner (:standard8)]

Ere, will you be landing this soon or does it need checkin-needed?

Comment 31

[Ere Maijala (slow)]

(In reply to comment #29)
> Ere, will you be landing this soon or does it need checkin-needed?

Looks like the tree is never good when I am...

Comment 32

[Ian Neal]

Comment on attachment 358587 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 32 & 37]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a4594a183cca

Comment 33

[Ian Neal]

Putting checkin-needed keyword back as cannot push to trunk yet as it is closed.

Comment 34

[Joe Sabash [:JoeS1]]

Maybe I'm missing something, but
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?changeset=a4594a183cca
Seems to have been checked in.

Comment 35

[Boris Zbarsky [:bzbarsky]]

That's the 1.9.1 branch, not trunk.

Comment 36

[Joe Sabash [:JoeS1]]

Ok just to verify that this fixes the problem in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090212 Lightning/1.0pre Shredder/3.0b2pre ID:20090212172422

Which is the main development "branch" for TB since we haven't officially branched yet.

Which goes to prove the old adage:
"A branch, by any other name, is still a "trunk"??

Shredder/3.1a1pre("pre-trunk") builds are presumably still affected.

Comment 37

[Serge Gautherie (:sgautherie)]

Comment on attachment 358587 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 32 & 37]


http://hg.mozilla.org/mozilla-central/rev/3f0053b5489f