Closed Bug 1485495 Opened Last year Closed Last year

Minor refactor in nsCSSFrameConstructor

Categories

(Core :: Layout, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

Details

Attachments

(2 files, 1 obsolete file)

I'd like to throw some small and reviewable patches while I'm in the journey in implementing column-span.
We can access the bool pref in C++ by writing

  if (StaticPrefs::layout_css_column_span_enabled()) {
     // do something
  }
The original in/out parameter aNewFrame is confusion in its naming and the
logic around it. It should be clearer to rename aNewFrame to aBlockFrame,
and return the outermost frame created in ConstructBlock().

Delete the comment (in the header)

  "|aContentParentFrame| should be null if it's really the same as
  |aParentFrame|"

because callers like ConstructScrollableBlockWithConstructor() already
violate this, and ConstructBlock() dosen't rely on this invariant.


Depends On D4021
Attachment #9003301 - Attachment is obsolete: true
Comment on attachment 9003299 [details]
Bug 1485495 - Move column-span preference to StaticPrefList.h.

Daniel Holbert [:dholbert] has approved the revision.
Attachment #9003299 - Flags: review+
Comment on attachment 9003300 [details]
Bug 1485495 - Use AutoRestore to restore aState.mAdditionalStateBits.

Daniel Holbert [:dholbert] has approved the revision.
Attachment #9003300 - Flags: review+
r=me on the two non-obsolete patches here. Thanks for these cleanups & sorry for the delay!
Thanks for the review!
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cb7af446b841
Move column-span preference to StaticPrefList.h. r=dholbert
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c673b6f64457
Use AutoRestore to restore aState.mAdditionalStateBits. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/cb7af446b841
https://hg.mozilla.org/mozilla-central/rev/c673b6f64457
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.