Since toolkit's printUtils.js has started as a clone of communicator's printing.js and has the same functions defined, we should use that global file instead of the toolkit file. This affects Thunderbird files as well as a SeaMonkey mail, browser and composer.

Unfortunately, printUtils.js isn't as much a drop-in replacement for printing.js as I hoped, I'll need to investigate the diff between the two files.

The two differences I know about are the easy need to switch all calls to NSPrint and NSPrintSetup to call PrintUtils.print and PrintUtils.showPageSetup, and the not so easy need to deal with bug 270235 (particularly since the #ifdef MOZ_THUNDERBIRD bit has rotted, since BrowserExitPrintPreview() now only exists in calendar/, and to maximize the fun, Thunderbird was trying to avoid calls to hide the tabstrip, and now Thunderbird has a tabstrip (though not exactly the same tabstrip), while I think SeaMonkey mail doesn't, yet).

And perhaps the bug 246995 problem of the var XUL_NS in printUtils.js conflicting with a const XUL_NS, which should probably be solved by putting it in the PrintUtils object instead of the global scope.

Oh, the tabstrip part is easy: that should have been in browser.js's toggleAffectedChrome(), the way it is in suite's browser.js, so the ifdef bits should collapse into just needing a generalized way to get "the thing before which the toolbar should be inserted" and having Tbird close the window in its aExitPPCallback.

Bowing to the inevitable.

See bug 321954 where I did submit a patch to do something similar

Heh, somehow I missed that, even though I've been porting back your fixes from bug 311028 where it's mentioned.

Depends on the patch in bug 321954.

Comment on attachment 272380 [details] [diff] [review] Switch over SeaMonkey I think there will be the function getEngineWebBrowserPrint() that can be removed from: /editor/ui/composer/content/editor.js /mailnews/base/resources/content/msgPrintEngine.js /suite/browser/browser.js Also the method nsMsgPrintEngine::GetWebBrowserPrint could be removed from /mailnews/base/src/nsMsgPrintEngine.cpp if getWebBrowserPrint: function in printUtils.js works for mailnews/TB On a separate note should we also be comparing the two printPreviewBindings.xml files in suite and toolkit?

Ah, indeed. More things to delete are always good, though I only hope I've gone after nsMsgPrintEngine correctly. I'm not sure about the printPreviewBindings - unless I missed something, the difference is just a little code cleanup, and some debug code. Is it worth fighting for scarce review resources to sync and consolidate the two right now?

Comment on attachment 272457 [details] [diff] [review] Switch over SeaMonkey, v2 On second thought, "please review this even though you have no idea what will wind up happening in bug 321954" isn't a particularly good idea.

Comment on attachment 272379 [details] [diff] [review] Switch over Tbird Poor Magnus is going to be heartbroken that he missed a chance to review printing code by being offline.

Just in case I forget, but then look at this comment, the Tbird patch does depend on the SeaMonkey one landing first.

Comment on attachment 272457 [details] [diff] [review] Switch over SeaMonkey, v2 Looks good to me, sorry for the delay, r=me

Just a reminder that calPrintEngine.js still mentions bug 270235 and probably could do with some tidy up now that bug 321954 has landed either in this bug or a follow up one (unless there is an appropriate one already filed).

mail/base/content/msgPrintEngine.xul 1.6 mail/base/content/mailWindowOverlay.xul 1.221 mail/base/content/mail3PaneWindowCommands.js 1.40 mail/base/content/messageWindow.js 1.52 mail/components/addrbook/content/addressbook.xul 1.71 mail/components/compose/content/messengercompose.xul 1.105 mail/components/compose/content/MsgComposeCommands.js 1.117 editor/ui/composer/content/ComposerCommands.js 1.206 editor/ui/composer/content/editor.xul 1.160 editor/ui/composer/content/editor.js 1.244 mailnews/compose/resources/content/MsgComposeCommands.js 1.401 mailnews/compose/resources/content/messengercompose.xul 1.291 mailnews/addrbook/resources/content/addressbook.xul 1.180 mailnews/base/resources/content/msgPrintEngine.xul 1.19 mailnews/base/resources/content/msgPrintEngine.js 1.23 mailnews/base/src/nsMsgPrintEngine.cpp 1.92 mailnews/base/public/nsIMsgPrintEngine.idl 1.11 suite/browser/navigator.xul 1.452 suite/browser/navigatorOverlay.xul 1.329 suite/browser/viewSourceOverlay.xul 1.30 suite/browser/browser.js 1.41 suite/common/jar.mn 1.26 suite/common/printing.js delete

Gah, and mailnews/compose/resources/content/MsgComposeCommands.js 1.402, since I somehow managed to commit just one of the two hunks in that file.

(In reply to comment #17) > Just a reminder that calPrintEngine.js still mentions bug 270235 and probably > could do with some tidy up now that bug 321954 has landed either in this bug or > a follow up one (unless there is an appropriate one already filed). Bug 350099 would be the appropriate one - see its antecedent, bug 275883, which made that deadcode.