983581 - Clean up handling of columns transitioning from balanced to non-balanced due to overflow

Status: NEW

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's broken. The testcase of bug 983465 comment #0, with that bug fixed, shows that we don't correctly break out of balanced mode and create overflowing columns in the horizontal direction.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Unfortunately I can't create a reduced testcase. Hmm.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Mats Palmgren (inactive)]

Comment on attachment 8391119 [details] [diff] [review]
fix

Is this the right patch?  It fails to compile:

nsColumnSetFrame.cpp:325:10: error: no member named 'mComputedMaxHeight' in 'nsColumnSetFrame::ReflowConfig':
  config.mComputedMaxHeight = computedMaxHeight;
         ^~~~~~~~~~~~~~~~~~

Nit: nsSplittableFrame::GetEffectiveComputedMaxHeight (and same also for
GetEffectiveComputedHeight):

>  if (aConsumedHeight == NS_INTRINSICSIZE) {
>    aConsumedHeight = GetConsumedHeight();
>  }
>
>  height -= aConsumedHeight;
>
>  if (aConsumedHeight != 0 && aConsumedHeight != NS_INTRINSICSIZE) {
>    // We just subtracted our top-border padding, since it was included in the

GetConsumedHeight() always return a real height, not NS_INTRINSICSIZE,
so the "&& aConsumedHeight != NS_INTRINSICSIZE" seems redundant.
Also, "top-border padding" doesn't read very well, perhaps "top border+padding"?
or "top-border/padding"?

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8391119 [details] [diff] [review]
fix

I'll wait for a new patch before reviewing this further.