Audit whether MOZ_CAN_RUN_SCRIPT annotations for nsPrintJob functions are correct/necessary or not
Categories
(Core :: Printing: Setup, task)
Tracking
()
People
(Reporter: hiro, Unassigned)
References
Details
(Whiteboard: [print2020])
For example, nsPrintJob::SetupToPrintContent has MOZ_CAN_RUN_SCRIPT, indeed it flushes pending styles/layout whatever, BUT in the function we flush the static-cloned documents which means there is no script in the documents unless I am misunderstanding what CreateStaticClone does.
Reporter | ||
Comment 2•4 years ago
|
||
Some relevant questions;
- Window.print() should explicitly flush the document? (It seems not to me IIUC the spec), but..
- nsGlobalWindowInner::Print should be annotated as MOZ_CAN_RUN_SCRIPT since it fires beforeprint/afterprint events?
Comment 3•4 years ago
|
||
If the MOZ_CAN_RUN_SCRIPT
is really safe, it may be fine to change it to MOZ_CAN_RUN_SCRIPT_BOUNDARY
with comment explaining intentionally changed to it.
Comment 4•4 years ago
|
||
(but I don't know whether it's truly safe or not.)
Reporter | ||
Comment 5•4 years ago
|
||
Thank you, masayuki. I am mostly 100% sure functions in nsPrintJob shouldn't run script at all because they are used for the static-cloned documents. One thing I am wondering is whether Document::CreateStaticClone should flush pending styles or not (currently it doesn't), and I suppose it shouldn't since users want to see results what the user is seeing at that moment.
Reporter | ||
Comment 6•4 years ago
|
||
Hmm nsPrintJob::TurnScriptingOn turns on/off scripts in the cloned documents. It's confusing to me. Do we really want to run scripts in the cloned documents?
Reporter | ||
Comment 7•4 years ago
|
||
Olli, at the time you introduced nsScriptSuppressor machinery in bug 424377 we hadn't cloned documents for printing, but we've cloned documents for printing since you fixed bug 487667, now we don't need the script turn on/off machinery at all? Am I understanding correctly?
Comment 8•4 years ago
|
||
Since it has been so long after those bugs, lots of the underlying code has changed possibly significantly, so one will need to check what all
https://searchfox.org/mozilla-central/rev/501eb4718d73870892d28f31a99b46f4783efaa0/layout/printing/nsPrintJob.cpp#2843,2858 do.
And with Fission I assume some of the clone-doc-for-printing will need to become asynchronous, so disabling scripting temporarily will be probably needed - maybe.
Reporter | ||
Comment 9•4 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #8)
Since it has been so long after those bugs, lots of the underlying code has changed possibly significantly, so one will need to check what all
https://searchfox.org/mozilla-central/rev/501eb4718d73870892d28f31a99b46f4783efaa0/layout/printing/nsPrintJob.cpp#2843,2858 do.
Thank you Olli, I will check the what Suspend() and Resume() do.
And with Fission I assume some of the clone-doc-for-printing will need to become asynchronous, so disabling scripting temporarily will be probably needed - maybe.
Yep, that's the initial motivation on this. (I am assuming you mean disabling scripting temporarily in original documents by the time we finished cloning all descendant documents).
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•3 years ago
|
Description
•