Closed Bug 1145323 Opened 8 years ago Closed 1 year ago

nsHTMLReflowMetrics instances in nsFlexContainerFrame.cpp and should be given container's reflow state (not child's)

Categories

(Core :: Layout: Flexbox, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: dholbert, Assigned: TYLin)

Details

Attachments

(1 file)

In nsFlexContainerFrame.cpp right now, whenever we reflow a child, we create its nsHTMLReflowMetrics like this:
> 3875   nsHTMLReflowMetrics childDesiredSize(childReflowState);

Specifically: we pass in the *child's* reflow-state to the reflow-metrics constructor.

But, it looks like most other code passes in the *container's* reflow state. e.g. here in nsBlockFrame::Reflow():
> 1224     // Reflow the bullet
> 1225     nsHTMLReflowMetrics metrics(aReflowState);
[...[
> 1235     ReflowBullet(bullet, state, metrics, lineBStart);
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?rev=8949ff85977e#1225

Note that we're passing in the *block's* reflow state to the 'metrics' constructor, whereas we're passing in the child's reflow state ('state') to the Reflow call.


One more example:
> 145   nsHTMLReflowState reflowState(aPresContext, aReflowState,
> 146                                 aBarFrame, availSize);
[...]
> 196   nsHTMLReflowMetrics barDesiredSize(aReflowState);
> 197   ReflowChild(aBarFrame, aPresContext, barDesiredSize, reflowState, xoffset,
> 198               yoffset, 0, aStatus);
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsProgressFrame.cpp?rev=c04e1a0e1920#196
Here, "aReflowState" (passed into the metrics constructor) is the parent's reflow state, and is different from the reflow state that we pass into ReflowChild.

Anyway -- we should bring nsFlexContainerFrame into compliance with this.  I noticed one other similarly-broken spot in nsFieldSetFrame, too:
> 520     nsHTMLReflowMetrics kidDesiredSize(kidReflowState,
> 521                                        aDesiredSize.mFlags);
[...]
> 527     ReflowChild(inner, aPresContext, kidDesiredSize, kidReflowState,
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsFieldSetFrame.cpp?rev=70dda75a914f#520
(It looks like the reflow-state nsHTMLReflowMetrics constructor-param is only used for its writing-mode. So I think this only actually affects behavior when a child frame's writing-mode differs from its parent's writing-mode.)

ReflowOutput can accept any writing mode or ReflowInput passing to its
constructor, so a caller can pass a writing mode that is more convenient
to retrieve the size.

After reflowing flex items, we want to cache its metrics in flex item's
writing mode, so we pass item's ReflowInput into ReflowOutput. However,
for out-of-flow children, we can change ReflowOutput to use flex
container's writing mode by convention.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED

nsHTMLReflowMetrics is now ReflowOutput. According to ReflowOutput's comment, it can accept any writing mode. A caller can pass a writing mode which is convenient to access the size.

Component: Layout → Layout: Flexbox

Thanks for following up on this!

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0be9d8da537d
Audit callers of ReflowOutput constructor in nsFlexContainerFrame. r=dholbert
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.