Bug 127565

Summary: [New Multicolumn] Eliminate RenderMultiColumnBlock
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, rakuco, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch koivisto: review+

Description Dave Hyatt 2014-01-24 10:22:02 PST
Fold the RenderMultiColumnBlock back into RenderBlockFlow so that all block flows have multi-column capability (including table cells, list items and the RenderView).
Comment 1 Dave Hyatt 2014-01-24 10:27:57 PST
Created attachment 222117 [details]
Patch
Comment 2 WebKit Commit Bot 2014-01-24 10:30:36 PST
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.
Comment 3 Antti Koivisto 2014-01-24 10:35:29 PST
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
Comment 4 Dave Hyatt 2014-01-24 11:51:16 PST
Landed in r162712 (with additional fix for GTK makefile).