Bug 127565 – [New Multicolumn] Eliminate RenderMultiColumnBlock

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dave Hyatt]

Fold the RenderMultiColumnBlock back into RenderBlockFlow so that all block flows have multi-column capability (including table cells, list items and the RenderView).

[Dave Hyatt]

Created attachment 222117 [details]
Patch

[WebKit Commit Bot]

Attachment 222117 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderBlockFlow.cpp:3200:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/rendering/RenderBlockFlow.cpp:3208:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antti Koivisto]

Comment on attachment 222117 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222117&action=review

> Source/WebCore/rendering/RenderBlockFlow.cpp:3214
> +            if (minColumnCount >= desiredColumnCount) {
> +                // The forced page breaks are in control of the balancing.  Just set the column height to the
> +                // maximum page break distance.
> +                if (!pageLogicalHeight) {
> +                    LayoutUnit distanceBetweenBreaks = std::max<LayoutUnit>(colInfo->maximumDistanceBetweenForcedBreaks(),
> +                        view().layoutState()->pageLogicalOffset(this, borderAndPaddingBefore() + layoutOverflowLogicalBottom) - colInfo->forcedBreakOffset());
> +                    columnHeight = std::max(colInfo->minimumColumnHeight(), distanceBetweenBreaks);
> +                }

This is getting deep, helper functions might be useful.

> Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp:125
> -    RenderMultiColumnBlock* parentBlock = toRenderMultiColumnBlock(parent());
> +    RenderBlockFlow* parentBlock = toRenderBlockFlow(parent());

i would just use auto, toRenderBlockFlow already names the type. 
parentFlow would be a more informative variable name.

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:48
> -    RenderMultiColumnBlock* multicolBlock = toRenderMultiColumnBlock(parent());
> +    RenderBlockFlow* multicolBlock = toRenderBlockFlow(parent());

same here, 'multicolumnFlow'

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:202
> +    RenderBlockFlow* parentBlock = toRenderBlockFlow(parent());

same here

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:224
> -    RenderMultiColumnBlock* multicolBlock = toRenderMultiColumnBlock(parent());
> +    RenderBlockFlow* multicolBlock = toRenderBlockFlow(parent());

same here

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:268
> +    RenderBlockFlow* parentBlock = toRenderBlockFlow(parent());

same here

[Dave Hyatt]

Landed in r162712 (with additional fix for GTK makefile).