893548 - Print dialogue from main 3pane disappears immediately into background (hidden behind main window as focus is forced back into main window, preventing average users from printing)

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Thomas D. (:thomas8)]

STR (seen on 25.0a1 (2013-07-14), WinXP)

0) start out from message list & msg preview in main 3pane window
1) select a msg from msg list
2) trigger Print command (e.g. Ctrl+P, or File > Print)

Actual Result

- Print dialogue shortly appears on top of 3pane as expected, but then immediately disappears to hide behind the main 3pane window.
- Msg in main 3pane is focused as if nothing had happened.
- For average user, this will make it impossible to print their received messages at all -> MAJOR!

This should block the release - can someone please set the right flag to request that?

Expected result

- Print dialogue should stay in front / on top of main 3pane window, and have focus

I only saw this since maybe 2 or 3 weeks ago, might be a recent regression. But I'm not printing often enough from Trunk to be sure.
Note: works for *composition* windows.

Comment 1

[Wayne Mery (:wsmwk)]

I see this with 24.0a1 (2013-05-29) and not with 23.0a1 (2013-04-30). So broke somewhere in that ~30 days.

An annoyance. But definitely not impossible.

(Note: not broken for address book)

Comment 2

[Wayne Mery (:wsmwk)]

works 24.0a1 (2013-05-22) http://hg.mozilla.org/mozilla-central/rev/c21ef3664c67

fails 24.0a1 (2013-05-23) http://hg.mozilla.org/mozilla-central/rev/00b264c7cced

Comment 3

[Mark Banner (:standard8)]

Easier link regression range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c21ef3664c67&tochange=00b264c7cced

Is this broken on Firefox as well?

Comment 4

[Wayne Mery (:wsmwk)]

Not broken in Firefox.
Not broken in address book.
AFICT only message pane (standalone and preview)

Comment 5

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #1)
> I see this with 24.0a1 (2013-05-29) and not with 23.0a1 (2013-04-30). So
> broke somewhere in that ~30 days.
> 
> An annoyance. But definitely not impossible.

I maintain this is major because average users will have no idea where to look for the hidden print dialogue, which just disappears instantly without trace (nothing in Task bar either). So unless we want TB to become a laughing stock by instructing users how to foreground hidden print dialogues each time they print, this appears as a major loss of basic functionality for a majority of users, and a daily annoyance to advanced users. We shouldn't sink that low.

Given the single-day regression range in comment 3, it's certainly possible to hack that right. Any volunteers?

Comment 6

[Wayne Mery (:wsmwk)]

I'm not disputing major. And not disputing that some users might not get it. What I'm disputing is the idea that it's impossible.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: patch

I compared the window.openDialog() print calls of main window and address book. The only difference is the window argument.

I don't know what I'm doing and if this is the right way, but it works :).

Comment 8

[Magnus Melin [:mkmelin]]

Nothing stands out from the regression range checkins, though bug 396542 seems to be related to focus...

Comment 9

[Thomas D. (:thomas8)]

(In reply to Richard Marti [:Paenglab] from comment #7)
> Created attachment 792931 [details] [diff] [review]
> patch

Great, thanks! Will CC you more often ;)

> I don't know what I'm doing and if this is the right way, but it works :).

Fair enough... ;)
Can we double-check some details for "it works":
- for single msg, prints the right msg in right layout on right printer?
- for multiple selected msgs, prints all of them correctly?
- escaping from print dialogue returns focus to window where print was started?
- after showing print dialogue, or printing, Print *Preview* still works as expected?

Richard's patch removes the window argument, which will be evaluated here:

http://mxr.mozilla.org/comm-central/source/mailnews/base/content/msgPrintEngine.js#168

168     if (window.arguments.length > 5) {
169       printEngine.setParentWindow(window.arguments[5]);
170     } else {
171       printEngine.setParentWindow(null);
172     }

So after the patch, we set ParentWindow to (null) instead of (window).
Well then, if it works like that...

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 792931 [details] [diff] [review]
patch

Review of attachment 792931 [details] [diff] [review]:
-----------------------------------------------------------------

So that window sets a parent window for the printing...
http://mxr.mozilla.org/comm-central/source/mailnews/base/content/msgPrintEngine.js#168

... which sounds like it could be useful.

Anybody checked if seamonkey trunk has this regression? At least they have the same call we do.
But thunderbird apparently lacks a "return true;" at the end of PrintEnginePrintInternal. Could that cause something? 
Also i notice Print Preview doesn't work at all, at least on trunk :/

Comment 11

[Magnus Melin [:mkmelin]]

Filed bug 907338 for that.

Comment 12

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin from comment #10)
> Also i notice Print Preview doesn't work at all, at least on trunk :/

I saw it too. I checked TB 24 and the preview is working.

Comment 13

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #6)
> I'm not disputing major. And not disputing that some users might not get it.
> What I'm disputing is the idea that it's impossible.

Sorry, didn't notice the link to the original description. Sure, printing is still technically possible even with this bug present, just hard to find for average user, *almost* impossibly hard... ;)

Comment 14

[Thomas D. (:thomas8)]

Browser cache has it's own life, sorry

Comment 15

[jlerner10]

I'd like to add to this the following.

When I go to print am email the Print window screen ends up behind an open program.
I have uninstalled TB and reinstalled it.

Any way to fix this problem?

What I mean about a Print Dialog
When you click Print a window opens and the you can click Print for the printer to print the email.

If I do a Print Preview then Print the Print window shows up in front of any open program like it should.

All other print jobs are normal.

Is this a bug or a problem on my end?


Windows 7 Pro SP1
Thunderbird 24.0b1
Posted the same at
http://forums.mozillazine.org/viewtopic.php?f=29&t=2743313&p=13043645#p13043645

Comment 16

[Richard Marti (:Paenglab)]

(In reply to jlerner10 from comment #15)
> I'd like to add to this the following.
> 
> When I go to print am email the Print window screen ends up behind an open
> program.
> I have uninstalled TB and reinstalled it.
> 
> Any way to fix this problem?

The only way is to wait until this bug is fixed and a new beta of TB 24 with this patch including is built.

Comment 17

[jlerner10]

Thanks.

I have gone back to TB23 for now.
I will either wait for the next TB24 revision or version or install TB24 since I know what is going on and deal with it for now.

Comment 18

[Thomas D. (:thomas8)]

Magnus, this is one-word-patch is waiting for your review to land, as a matter of urgency. Not landing this bugfix for TB24 has the potential to make Thunderbird a laughing stock, and an application that can't even handle a simple print dialogue correctly will certainly lose out in terms of ux-trust, not to mention the support hassle that will be flooding us if we don't fix this.

Could you please review this as a matter of urgency, and then request approval-comm-beta + aurora on the patch?

(In reply to Magnus Melin from comment #10)
> Comment on attachment 792931 [details] [diff] [review]
> patch
> Review of attachment 792931 [details] [diff] [review]:
> -----------------------------------------------------------------
> So that window sets a parent window for the printing...
> http://mxr.mozilla.org/comm-central/source/mailnews/base/content/
> msgPrintEngine.js#168
> 
> ... which sounds like it could be useful.

Agreed, but as other print windows in TB successfully live and work without setting the parent window, it does not seem to matter at all.

As this simple patch fixes what we want to fix, and as it apparently prints the currently selected message(s) correctly as it should per comment 7, that's more important than any other theoretical edge-case problem which not setting the parent window might cause. Richard, can you confirm that with your patch, it prints exactly the selected message(s) correctly, both for single message and multiple messages?

> Anybody checked if seamonkey trunk has this regression? At least they have
> the same call we do.
> But thunderbird apparently lacks a "return true;" at the end of
> PrintEnginePrintInternal. Could that cause something? 

I suggest that we refrain from exploring other options in other applications given that the patch provides a working option which is completely congruent with the same function calls elsewhere in Thunderbird.

> Also i notice Print Preview doesn't work at all, at least on trunk :/

Fortunately per Richard's comment 12, TB24 beta is not affected by that.

Comment 19

[Mark Banner (:standard8)]

(In reply to Magnus Melin from comment #10)
> But thunderbird apparently lacks a "return true;" at the end of
> PrintEnginePrintInternal. Could that cause something? 
> Also i notice Print Preview doesn't work at all, at least on trunk :/

Tracing through the calls, the return value doesn't actually get used. So there's no effect there.

(In reply to Magnus Melin from comment #10)
> So that window sets a parent window for the printing...
> http://mxr.mozilla.org/comm-central/source/mailnews/base/content/
> msgPrintEngine.js#168
> 
> ... which sounds like it could be useful.

I've traced this through into nsMsgPrintEngine.cpp.

There's two places where the window argument is used. One is in PrintMsgWindow, however, that's redundant (bug 909687).

The other one is in ShowProgressDialog - the use here would attach a progress dialog to the supplied window argument. In this case the progress dialog is for "LoadingMailMsgForPrintPreview" or "LoadingMailMsgForPrint", so what it looks like is:

1) User makes a call to print
2) nsMsgPrintEngine puts up a progress dialog
3) Once the print engine is ready, the progress dialog is hidden, and the settings displayed (or some order - I can't find the bits in the code at the moment).

What I think is happening is that in the 3-pane window case, the progress dialog is now stealing the focus back to the 3-pane window, and then isn't getting it right when the print dialog is opened.

I can't see anything obvious in the regression range that could cause this.

In summary, AFAICT the only affect of this change is for if the progress dialog is put up in front of a specific window or not, so I think it is fine to take it as is.

Comment 20

[Magnus Melin [:mkmelin]]

Comment on attachment 792931 [details] [diff] [review]
patch

Review of attachment 792931 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, sounds reasonable. r=mkmelin

Comment 21

[Richard Marti (:Paenglab)]

Clearing the needinfo as Mark's explanation should be enough. Not able to try this patch now at work but at home the printout of a mail with and without patch was looking the same.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/7c728ea3bda0

Comment 23

[Mark Banner (:standard8)]

Comment on attachment 792931 [details] [diff] [review]
patch

Yeah, I think this is safe. a=Standard8

Comment 24

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/76aee4d7b4ab
https://hg.mozilla.org/releases/comm-beta/rev/c0809b185d4c