Investigate unused return values in printProgress dialog
Categories
(Toolkit :: Printing, enhancement, P3)
Tracking
()
People
(Reporter: jallmann, Unassigned)
References
Details
Work on Bug 1529231 revealed that return values from the onCancel function of the printProgress.xul dialog were never actually checked.
Their removal as part of the work on Bug 1529231 therefore shouldn't break anything, but the initial intention of the return statements should be investigated to make sure everything works as expected.
|
||
Comment 1•5 years ago
|
||
Mike, can you check what's going on here?
To be clear, this code has comments suggesting that it's trying to affect whether the dialog closes.
But the dialog code that calls onCancel() doesn't pass on the return value, so it has no effect.
Should the code actually be preventing the dialog from closing?
|
||
Comment 2•5 years ago
|
||
Sorry for the delay - had to refresh my memory of how this code works. It's a bit of a mess, to put it mildly.
On construction of printProgress.xul, printProgress.js registers a listener on a PrintProgress object to hear updates on how the print / print preview job is going.
Looking at onCancel(), it looks like what occurs is that we tell the PrintProgress object that we've been user-cancelled. This makes it so that any further attempts to register listeners on that PrintProgress object will cause that listener to hear that the job is finished (even if it's not actually finished). That's all.
And if for some reason that throws an exception (which I can't see it doing - we're flipping a bool there), then we're supposed to tell the dialog to close right away. Otherwise, it looks like we're supposed to wait for the PrintProgress object to send the STATE_STOP update so that we close ourselves here.
I can't think of anything bad that would happen if the dialog continues to go away early. When the window unloads, it unregisters its listener here: https://searchfox.org/mozilla-central/rev/7c20ad925005fbad7b8e08813115f1ec7fa1c248/toolkit/components/printing/content/printProgress.js#194
So I think continuing to ignore this and closing the dialog right away is probably benign.
|
|
||
Comment 3•5 years ago
|
||
Excellent, thanks Mike!
Description
•