Printing a single address book contact is broken

RESOLVED FIXED in Thunderbird 53.0

Status

Thunderbird
Address Book
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

Trunk
Thunderbird 53.0

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 months ago
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]
(Assignee)

Comment 1

7 months ago
Created attachment 8824738 [details] [diff] [review]
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8824738 - Flags: review?(acelists)

Comment 2

7 months ago
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.
(Assignee)

Comment 3

7 months ago
OK, let me try let selectionArray = [];
New patch coming.
(Assignee)

Comment 4

7 months ago
Created attachment 8824794 [details] [diff] [review]
1319409-undefined.patch (v2).
Attachment #8824738 - Attachment is obsolete: true
Attachment #8824738 - Flags: review?(acelists)
Attachment #8824794 - Flags: review?(acelists)

Comment 5

7 months ago
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.
Attachment #8824794 - Flags: review?(acelists) → review+
(Assignee)

Comment 6

7 months ago
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.
Keywords: checkin-needed
Version: 52 Branch → Trunk
(Assignee)

Comment 7

7 months ago
https://hg.mozilla.org/comm-central/rev/d9c51a68b3730c824393c84539025cd9296ca581
Blocks: 1319409
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Comment 8

7 months ago
I tested printing a single card in a Daily opt build, it works.
(Assignee)

Comment 9

7 months ago
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.
(Assignee)

Updated

7 months ago
Keywords: checkin-needed
(Assignee)

Updated

7 months ago
Attachment #8824794 - Flags: approval-comm-aurora+
(Assignee)

Updated

6 months ago
status-thunderbird52: --- → fixed
status-thunderbird53: --- → fixed
You need to log in before you can comment on or make changes to this bug.