1485495 - Minor refactor in nsCSSFrameConstructor

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P4)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I'd like to throw some small and reviewable patches while I'm in the journey in implementing column-span.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485495 - Move column-span preference to StaticPrefList.h.

We can access the bool pref in C++ by writing

  if (StaticPrefs::layout_css_column_span_enabled()) {
     // do something
  }

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485495 - Use AutoRestore to restore aState.mAdditionalStateBits.

Depends On D4020

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485495 - Make ConstructBlock() return the outermost frame created, and improve the document.

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

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 9003299 [details]
Bug 1485495 - Move column-span preference to StaticPrefList.h.

Daniel Holbert [:dholbert] has approved the revision.

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 9003300 [details]
Bug 1485495 - Use AutoRestore to restore aState.mAdditionalStateBits.

Daniel Holbert [:dholbert] has approved the revision.

Comment 6

[Daniel Holbert [:dholbert]]

r=me on the two non-obsolete patches here. Thanks for these cleanups & sorry for the delay!

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Thanks for the review!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb7af446b841
Move column-span preference to StaticPrefList.h. r=dholbert

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c673b6f64457
Use AutoRestore to restore aState.mAdditionalStateBits. r=dholbert

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/cb7af446b841
https://hg.mozilla.org/mozilla-central/rev/c673b6f64457