Closed Bug 1245978 Opened 5 years ago Closed 5 years ago

de-COM the nsDocumentViewer::CreateStyleSet method


(Core :: Layout, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: dholbert, Assigned: dholbert)




(3 files)

Noticed while reviewing bug 1244068 that nsDocumentViewer::CreateStyleSet has an unnecessary (always-NS_OK) return value.

Filing this bug on dropping that return value, and just directly returning the thing it creates.

Wrote a patch for this locally (dropping the return value at least, in this function & its interface & its callers), and it compiles, so I think this doable.

I'll layer this on top of bug 1244068 so as not to bitrot heycam.
The first patch here makes a line in NS_DECL_NSIDOCUMENTVIEWERPRINT over 80 characters.

This second patch brings that line back underneath 80 characters by dropping the unnecessary "virtual" keyword on that line. (It's unnecessary because we already have "override", which requires that the method is virtual.)

This brings this macro into alignment with our style guide, which (since May of last year) says "Method declarations must use at most one of the following keywords: virtual, override, or final."

(For background, this coding style guideline was introduced in this dev.platform thread:!msg/ )
Attachment #8723715 - Flags: review?(cam)
This 3rd part does some whitespace cleanup in the NS_DECL_NSIDOCUMENTVIEWERPRINT macro. (The methods are all indented to align with this one method that used to have "nsresult" as its return value -- but now it no longer returns nsresult, so the indentation is insufficient & not really useful.)
Attachment #8723716 - Attachment description: part 3: drop now-unnecessary whitespace from NS_DECL_NSIDOCUMENTVIEWERPRINT macro → part 3: Drop unnecessary whitespace in NS_DECL_NSIDOCUMENTVIEWERPRINT macro. (whitespace only, no review)
Attachment #8723713 - Flags: review?(cam) → review+
Comment on attachment 8723715 [details] [diff] [review]
part 2: drop unnecessary (& style-guide-violating) 'virtual' keyword from  NS_DECL_NSIDOCUMENTVIEWERPRINT macro

Review of attachment 8723715 [details] [diff] [review]:

::: layout/base/nsIDocumentViewerPrint.h
@@ +80,5 @@
> +  bool     IsInitializedForPrintPreview() override; \
> +  void     InitializeForPrintPreview() override; \
> +  void     SetPrintPreviewPresentation(nsViewManager* aViewManager, \
> +                                       nsPresContext* aPresContext, \
> +                                       nsIPresShell* aPresShell) override;

Feel free, if you wish, to remove the extra space after those voids and bools.
Attachment #8723715 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) (away Feb 27 – Mar 13) from comment #4)
> Feel free, if you wish, to remove the extra space after those voids and
> bools.

I see you did this in the next patch. :-)
You need to log in before you can comment on or make changes to this bug.