Closed Bug 1313386 Opened 4 years ago Closed 4 years ago

Clean up the beforeprint/afterprint event dispatching code

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I have a patch to clean up the beforeprint/afterprint event dispatching code a little. Mostly it's a little about removing some unnecessary clutter from nsDocumentViewer's interface, but it also changing names to make it a bit clearer what's happening when skimming the code (adds "Auto" to names to make the RAII behavior clearer) and adds some documentation and links.
Attached patch patchSplinter Review
Attachment #8805152 - Flags: review?(dholbert)
Comment on attachment 8805152 [details] [diff] [review]
patch

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

r=me -- one requested followup:

::: layout/base/nsDocumentViewer.cpp
@@ +423,5 @@
> +    DispatchEventToWindowTree(mTop, NS_LITERAL_STRING("afterprint"));
> +  }
> +
> +private:
> +  static void DispatchEventToWindowTree(nsIDocument* aDoc,

Now that this is "private", and the callers always pass "mDoc" as the aDoc arg:  could you simplify this to make it non-static and just have it use |mDoc| directly?

(This probably belongs in a "part 2" patch, so that this first patch can have the nice property that it's just renaming/moving code without rewriting stuff at the same time.)
Attachment #8805152 - Flags: review?(dholbert) → review-
Comment on attachment 8805152 [details] [diff] [review]
patch

(Sorry, I meant to grant r+, not r-)
Attachment #8805152 - Flags: review- → review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d98eac568db
part 1 - Clean up the beforeprint/afterprint event dispatching code. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/23f27301062d
part 2 - Make AutoPrintEventDispatcher::DispatchEventToWindowTree non-static. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/2d98eac568db
https://hg.mozilla.org/mozilla-central/rev/23f27301062d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.