1654548 - (tkprint820) Ensure that XPCOMUtils is available in printUtils.js: Fix printing for MailNews Core.

Status: VERIFIED FIXED

(Toolkit :: Printing, defect)

Description

[[e-mail]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.89 Safari/537.36

Steps to reproduce:

alt f (for file)
p for print

same issue when using print button in task bar

Actual results:

white screen appeared

circle spinning top left corner

frozen

Expected results:

print dialouge should have appeared

and ability to print from there.

Comment 1

[Wayne Mery (:wsmwk)]

version 78? Or 80? If 80, did it break within the last 24 hours?

Comment 2

[WaltS48 [:walts48]]

Version 80.0 BuildID: 20200722095839 on Ubuntu 18.04.4 LTS.

Printing an email the Printer dialog is blank. I don't see a spinning circle.
Printing from Calendar I see a Print Preview, but clicking the Print button does not bring up the Printer dialog.

Also broken using BuildID: Build ID 20200721120747

Version 78.0.1 works as expected.

Comment 3

[Alice0775 White]

Error console when press Ctrl+P:

    <anonymous> chrome://global/content/printUtils.js:61
printUtils.js:61:1
    <anonymous> chrome://global/content/printUtils.js:61
Uncaught TypeError: can't access property "getPrintSettings", PrintUtils is undefined
    PrintEngineCreateGlobals chrome://messenger/content/msgPrintEngine.js:30
    OnLoadPrintEngine chrome://messenger/content/msgPrintEngine.js:21
    onload chrome://messenger/content/msgPrintEngine.xhtml:1
msgPrintEngine.js:30:3
    PrintEngineCreateGlobals chrome://messenger/content/msgPrintEngine.js:30
    OnLoadPrintEngine chrome://messenger/content/msgPrintEngine.js:21
    onload chrome://messenger/content/msgPrintEngine.xhtml:1
XHRPOSThttps://www.googleapis.com/oauth2/v3/token
[HTTP/1.1 200 OK 68ms]

Comment 4

[Alice0775 White]

Regression window:

https://hg.mozilla.org/comm-central/pushloghtml?fromchange=07002808e1250913159f992fbd7f38c0c9f9536e&tochange=c4250407ea60851e1f15a869e81db327f4950ef5
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f6127ce5c74439b7468a36c08941f3ab7210930f&tochange=fa0ad42a026fa263626d6d3290caadab1630cb7d

Comment 5

[[e-mail]]

sorry for the delay

version 80.0a1 2020-07-22 currently running and still has the bug in it

happened aprox 4 days ago ?

Comment 6

[[e-mail]]

running windows 10 version 2004

Comment 7

[Jonathan Watt [:jwatt]]

Drive-by comment...

(In reply to Alice0775 White from comment #3)

Uncaught ReferenceError: XPCOMUtils is not defined

Presumably XPCOMUtils is already in scope in Firefox when toolkit/components/printing/content/printUtils.js loads, but not in Thunderbird. I guess either the context that loads printUtils.js in Thunderbird needs to add the following, or maybe it should be added to printUtils.js inself?

const { XPCOMUtils } = ChromeUtils.import(
  "resource://gre/modules/XPCOMUtils.jsm"
);

Comment 8

[[e-mail]]

i went looking to update the files manually to add the jsm link

and they dont seem to be found anywhere under thunderbird daily directory should i be looking elsewhere??

C:\Program Files\Thunderbird Daily>dir printutils.js /s/w/a/p
Volume in drive C has no label.
Volume Serial Number is D031-0F6E
File Not Found

C:\Program Files\Thunderbird Daily>dir print*.* /s/w/a/p
Volume in drive C has no label.
Volume Serial Number is D031-0F6E
File Not Found

C:\Program Files\Thunderbird Daily>

fyi

Comment 10

[Thomas D. (:thomas8)]

(In reply to [e-mail] from comment #8)

i went looking to update the files manually to add the jsm link
and they dont seem to be found anywhere under thunderbird daily directory should i be looking elsewhere??
C:\Program Files\Thunderbird Daily>dir print*.* /s/w/a/p

Hey Paul! Thanks for looking into this!
jwatt's hypothesis looks right.

The files you were looking for are in the daily directory, but they are zipped into omni.ja which has most of our frontend code.
For testing purposes, here's how to tweak frontend files (.xhtml/.js) in your live installation:

• Modify your daily command line:
  "C:\Program Files\Thunderbird Daily\thunderbird.exe" -no-remote -purgecaches -devtools -allow-downgrade

• Update Daily and restart to ensure updates are applied

• In Thunderbird Daily folder, rename omni.ja to omni.zip

• unpack its contents into the same folder of omni.ja (Thunderbird Daily)

• change unpacked source files in text editor

• restart Daily to test your changes

• use developer tools/error console for debugging (clear error console before testing your changes)

• Note: If an (updated) omni.ja is present, your flat/unpacked install with your changes will be ignored. Try this to prevent updates:
  Options > Updates > Daily Updates > Allow Daily to: Check for updates, but let me choose whether to install them
  From my observation, and in violation of this setting, Daily will force-update and change that pref within a few days. If your manual edits to source files have no effect, double check for unwanted presence of omni.ja - probably Daily has updated behind your back.

Comment 11

[Thomas D. (:thomas8)]

.

Comment 12

[Thomas D. (:thomas8)]

I have a fix.

Comment 13

[Thomas D. (:thomas8)]

(In reply to Jonathan Watt [:jwatt] from comment #7)

Drive-by comment...

Thanks much Johnathan!

(In reply to Alice0775 White from comment #3)

Uncaught ReferenceError: XPCOMUtils is not defined

Presumably XPCOMUtils is already in scope in Firefox when toolkit/components/printing/content/printUtils.js loads, but not in Thunderbird. I guess either the context that loads printUtils.js in Thunderbird needs to add the following, or maybe it should be added to printUtils.js inself?

Firefox only has one or two callers, so it doesn't matter for them.
Thunderbird and SeaMonkey have dozens of callers, and we wouldn't want to change and burden all of them with loading an extra object.
So I'd think the better place to fix this is is upstream / toolkit.

const { XPCOMUtils } = ChromeUtils.import(
  "resource://gre/modules/XPCOMUtils.jsm"
);

Comment 14

[Thomas D. (:thomas8)]

Attachment: 1654548_printUtils.toolkit.diff

Hey Dave (:mossop), toolkit bug 1652627 has broken printing in MailNews Core (Thunderbird and SeaMoney) for dozens of callers. This one-liner fixes the problem in toolkit.

Unfortunately, cannot request a traditional review here, but here's the patch anyway (still new to Phabricator).

(In reply to Thomas D. from comment #13)

Firefox only has one or two callers, so it doesn't matter for them.
Thunderbird and SeaMonkey have dozens of callers, and we wouldn't want to change and burden all of them with loading an extra object.
So I'd think the better place to fix this is is upstream / toolkit.

Comment 16

[:Gijs (he/him)]

r- because if XPCOMUtils is not defined the patch you added will still throw an exception that XPCOMUtils is not defined. You should use typeof to check things like this.

However, I think the perfect fix here is that XPCOMUtils should just be available everywhere. As that's probably not going to come in time for this bug, the slightly less perfect but still preferable fix here would be for windows that source printUtils.js to define XPCOMUtils themselves. Otherwise all of these utility files in toolkit are going to end up with the same kind of "maybe include this thing" code which is tedious and leads to lots of duplication.

Comment 17

[:Gijs (he/him)]

(In reply to Thomas D. from comment #13)

we wouldn't want to change and burden all of them with loading an extra object.

But that's exactly what your patch is trying to do anyway - you're just sticking it in a shared file instead of at the callsites, and in any case most of these windows are all loading this object anyway - they're just doing it after loading printUtils. AFAICT suite isn't even affected because they load chrome://communicator/content/contentAreaContextOverlay.xul which loads contentAreaUtils.js which defines XPCOMUtils if nothing else has.

They'll all end up loading the same object (jsm modules are only loaded once) so it's really not that big a deal. It's also not about callsites, but about window scopes where printUtils.js loads. AFAICT you can just move it after the contentAreaUtils.js loads in windows where that's not the case.

Comment 19

[Thomas D. (:thomas8)]

(In reply to :Gijs (he/him) from comment #17)

They'll all end up loading the same object (jsm modules are only loaded once) so it's really not that big a deal. It's also not about callsites, but about window scopes where printUtils.js loads. AFAICT you can just move it after the contentAreaUtils.js loads in windows where that's not the case.

That seems to be the case for messenger.xhtml, but it's still not working. So this needs somebody with deeper knowledge how these things interact.
https://searchfox.org/comm-central/rev/ea18e0f4074d618b9bc5917ec26b2e9d8ce5a02a/mail/base/content/messenger.xhtml#112,144

112: <script src="chrome://global/content/contentAreaUtils.js"/>
144: <script src="chrome://global/content/printUtils.js"/>

Comment 20

[:Gijs (he/him)]

(In reply to Thomas D. from comment #19)

(In reply to :Gijs (he/him) from comment #17)

They'll all end up loading the same object (jsm modules are only loaded once) so it's really not that big a deal. It's also not about callsites, but about window scopes where printUtils.js loads. AFAICT you can just move it after the contentAreaUtils.js loads in windows where that's not the case.

That seems to be the case for messenger.xhtml, but it's still not working. So this needs somebody with deeper knowledge how these things interact.

It's working fine in messenger.xhtml, but if you hit accel-p it opens the msgPrintEngine.xhtml window, which only sources printUtils.js and not contentAreaUtils.js.

You could either add contentAreaUtils.js (and hope there's no conflicts), or add an import statement to msgPrintEngine.js (next to the one for Services.jsm) and load that before loading printUtils.js (which, off-hand, because PrintUtils doesn't get used by msgPrintEngine.js until the load event fires, should be sufficient)

Comment 23

[:Gijs (he/him)]

AFAICT some of this has been broken anyway since bug 1602561 or thereabouts, as bug 1620950 seems to have been incomplete - the compose window's print code is broken despite PrintUtils loading fine:

Uncaught TypeError: topBrowser.print is not a function
    printWindow chrome://global/content/printUtils.js:297
    DoCommandPrint chrome://messenger/content/messengercompose/MsgComposeCommands.js:2572
    doCommand chrome://messenger/content/messengercompose/MsgComposeCommands.js:1017
    doCommand chrome://messenger/content/messengercompose/MsgComposeCommands.js:1162
    goDoCommand chrome://global/content/globalOverlay.js:101
    oncommand chrome://messenger/content/messengercompose/messengercompose.xhtml:1

this appears to be because the editor/compose window has an editor node and custom element, and the code here assumes that the browser / browsing context is embedded in a browser which has a print method, cf. https://searchfox.org/mozilla-central/rev/a315a1a0f09550e23e4590a77e74f36543315da3/toolkit/content/widgets/browser-custom-element.js#1713 . Copying the implementation there to the editor custom element is probably sufficient to fix this... but this isn't a new regression. :-\

Comment 24

[:Gijs (he/him)]

Also as noted on matrix, the calendar windows seem to have print UI that is always disabled and does nothing even if I fix the printUtils imports (though at least it does fix the page setup menu entry - though there's little point in having page setup if print is always disabled? I dunno what's going on there...).

Comment 25

[:Gijs (he/him)]

Attachment: Bug 1654548 - fix printing in compose, view source, mail viewer, address book and attempt to improve state for calendar

Comment 26

[:Gijs (he/him)]

Attachment: Bug 1654548 - add print API on editor binding so thunderbird compose windows can print, r?jwatt

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1396098a49f6
add print API on editor binding so thunderbird compose windows can print, r=jwatt

Comment 28

[:Gijs (he/him)]

Marking leave-open so we don't close this before the comm-central fix lands.

Comment 29

[Thomas D. (:thomas8)]

Awesome :Gijs, thank you so much for helping us all with fixing this! :-)

Comment 30

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1396098a49f6

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/332b0b3f26ff
fix printing in compose, view source, mail viewer, address book and attempt to improve state for calendar r=mkmelin

Comment 32

[:Gijs (he/him)]

Comment on attachment 9168165 [details]
Bug 1654548 - add print API on editor binding so thunderbird compose windows can print, r?jwatt

Beta/Release Uplift Approval Request

• User impact if declined: Thunderbird has no printing in compose windows
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This code is unused in mozilla-central, but it helps Thunderbird.
• String changes made/needed: Nope

Comment 33

[:Gijs (he/him)]

(In reply to Pulsebot from comment #31)

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/332b0b3f26ff
fix printing in compose, view source, mail viewer, address book and attempt
to improve state for calendar r=mkmelin

Magnus, I'm assuming the TB team handle uplift to comm-beta for the comm-central side of this patchset?

Comment 34

[Magnus Melin [:mkmelin]]

Is it going to mozilla-beta? If not, then we'll just let it ride the trains, as we don't do special m-c-beta branches unless it's something really important to get in fast.

Comment 35

[Magnus Melin [:mkmelin]]

Oh, I should have read the uplift request. We'll take care for comm-beta if landed on beta yes.

Comment 38

[Julien Cristau [:jcristau]]

Comment on attachment 9168165 [details]
Bug 1654548 - add print API on editor binding so thunderbird compose windows can print, r?jwatt

approved for 80.0b5

Comment 39

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/36662489e4ae

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/13503034ce48
c-c followup to fix linting in mailnews/base/content/msgPrintEngine.js. rs=eslint

Comment 41

[Magnus Melin [:mkmelin]]

Wayne - please approve uplift of these for comm-beta:
https://hg.mozilla.org/comm-central/rev/332b0b3f26ff
https://hg.mozilla.org/comm-central/rev/13503034ce48

Comment 42

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #41)

Wayne - please approve uplift of these for comm-beta:
https://hg.mozilla.org/comm-central/rev/332b0b3f26ff
https://hg.mozilla.org/comm-central/rev/13503034ce48

approved for comm-beta

Comment 44

[Magnus Melin [:mkmelin]]

Rob, please land the changeset from comment 42 in on comm-beta.

Comment 50

[Rob Lemley [:rjl]]

Thunderbird 80.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/f645a932dd65
https://hg.mozilla.org/releases/comm-beta/rev/db11b2b52f1f

Comment 53

[WaltS48 [:walts48]]

Print preview and printing of email and calendar worked in my test of the 80.0b2 release candidate on Ubuntu 18.04.4.