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)
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.
OK, let me try let selectionArray = ; New patch coming.
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+
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.
Version: 52 Branch → Trunk
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
I tested printing a single card in a Daily opt build, it works.
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.
You need to log in before you can comment on or make changes to this bug.