1329455 - Printing a single address book contact is broken

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Jorg K (CEST = GMT+2)]

First this was was broken by bug 1319409, see bug 1319409 comment #59, and when fixing the obvious JS error, I get:

[7532] WARNING: NS_ENSURE_TRUE(aMsgURI) failed: file c:/mozilla-source/comm-central/mailnews/base/src/nsMsgPrintEngine.cpp, line 294
JavaScript error: chrome://messenger/content/msgPrintEngine.js, line 180: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIMsgPrintEngine.addPrintURI]

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1319409-undefined.patch (v1)

OK, to the suggestion of using selectionArray.push() didn't work since the array was already created with a fixed length and push made it longer.

I noticed that when trying to use selectionArray.length, which caused the error in comment #0.

So this just reinstates the previous code. And while printing a card, I noticed a problem in mailnews, too, which I'm fixing here as well.

Comment 2

[:aceman]

My proposal was to create the array as empty and then you would push into.
let selectionArray = [];

I do not like the old code, because it preallocates the numSelected items in the array, but then if printCardUrl is falsy for some cards, some items in the array end up uninitialized. The code then must be careful to only even touch totalCard number of items and can't just use selectionArray.length instead of totalCard.
I still think my suggestion was valid.

But if you just was to put back the old code I could accept that.

Comment 3

[Jorg K (CEST = GMT+2)]

OK, let me try let selectionArray = [];
New patch coming.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1319409-undefined.patch (v2).

Comment 5

[:aceman]

Comment on attachment 8824794 [details] [diff] [review]
1319409-undefined.patch (v2).

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

Thanks, this implements my idea. So assuming you tested this and it works for you, I am happy with this version.

Comment 6

[Jorg K (CEST = GMT+2)]

Yes, I tested this, the JS errors don't show any more. However, printing crashed in the end with:

Assertion failure: mHasActivePage (We can't guarantee a valid DrawTarget), at c:/mozilla-source/comm-central/mozilla/gfx/thebes/PrintTarget.cpp:68
#01: nsDeviceContext::CreateRenderingContextCommon (c:\mozilla-source\comm-central\mozilla\gfx\src\nsdevicecontext.cpp:354)
#02: nsDeviceContext::CreateRenderingContext (c:\mozilla-source\comm-central\mozilla\gfx\src\nsdevicecontext.cpp:331)
#03: nsSimplePageSequenceFrame::PrintNextPage (c:\mozilla-source\comm-central\mozilla\layout\generic\nssimplepagesequenceframe.cpp:747)
#04: nsPrintEngine::PrintPage (c:\mozilla-source\comm-central\mozilla\layout\printing\nsprintengine.cpp:2722)
#05: nsPagePrintTimer::Run (c:\mozilla-source\comm-central\mozilla\layout\printing\nspageprinttimer.cpp:89)
#06: nsThread::ProcessNextEvent (c:\mozilla-source\comm-central\mozilla\xpcom\threads\nsthread.cpp:1213)
#07: NS_ProcessNextEvent (c:\mozilla-source\comm-central\mozilla\xpcom\glue\nsthreadutils.cpp:381)
#08: mozilla::ipc::MessagePump::Run (c:\mozilla-source\comm-central\mozilla\ipc\glue\messagepump.cpp:96)
#09: MessageLoop::RunHandler (c:\mozilla-source\comm-central\mozilla\ipc\chromium\src\base\message_loop.cc:232)
#10: MessageLoop::Run (c:\mozilla-source\comm-central\mozilla\ipc\chromium\src\base\message_loop.cc:212)
#11: nsBaseAppShell::Run (c:\mozilla-source\comm-central\mozilla\widget\nsbaseappshell.cpp:158)
#12: nsAppShell::Run (c:\mozilla-source\comm-central\mozilla\widget\windows\nsappshell.cpp:264)
#13: nsAppStartup::Run (c:\mozilla-source\comm-central\mozilla\toolkit\components\startup\nsappstartup.cpp:283)
#14: XREMain::XRE_mainRun (c:\mozilla-source\comm-central\mozilla\toolkit\xre\nsapprunner.cpp:4494)
#15: XREMain::XRE_main (c:\mozilla-source\comm-central\mozilla\toolkit\xre\nsapprunner.cpp:4623)
#16: XRE_main (c:\mozilla-source\comm-central\mozilla\toolkit\xre\nsapprunner.cpp:4714)
#17: do_main (c:\mozilla-source\comm-central\mail\app\nsmailapp.cpp:320)
#18: NS_internal_main (c:\mozilla-source\comm-central\mail\app\nsmailapp.cpp:455)
#19: wmain (c:\mozilla-source\comm-central\mozilla\toolkit\xre\nswindowswmain.cpp:118)
#20: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
#21: BaseThreadInitThunk[C:\Windows\system32\kernel32.dll +0x159cd]
#22: RtlUserThreadStart[C:\Windows\SYSTEM32\ntdll.dll +0x2a561]

But that's for another bug, I can't try it in a Daily since this doesn't work due to this bug.

Comment 7

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/d9c51a68b3730c824393c84539025cd9296ca581

Comment 8

[Jorg K (CEST = GMT+2)]

I tested printing a single card in a Daily opt build, it works.

Comment 9

[Jorg K (CEST = GMT+2)]

Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/06d0c74bf867ad62fc6c50f4e5724544522013bf (regressing bug)
https://hg.mozilla.org/releases/comm-aurora/rev/1e40d7501405429e0f88adefc3d096c59464dc56 (regression fix)
https://hg.mozilla.org/releases/comm-aurora/rev/1d819c377a51ebb29c4285930268734a2b3a6b6b (this bug here)

This needed uplift since the regressing bug was uplifted, see bug 1319409 comment #60.