Bug 94616 - [New Multicolumn] Get column rules painting
Summary: [New Multicolumn] Get column rules painting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-21 10:36 PDT by Dave Hyatt
Modified: 2012-08-21 11:15 PDT (History)
0 users

See Also:


Attachments
Patch (16.57 KB, patch)
2012-08-21 10:44 PDT, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2012-08-21 10:36:04 PDT
Get the column rules painting properly in column sets.
Comment 1 Dave Hyatt 2012-08-21 10:44:36 PDT
Created attachment 159721 [details]
Patch
Comment 2 Simon Fraser (smfr) 2012-08-21 11:02:27 PDT
Comment on attachment 159721 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        No new tests (OOPS!).

OOPS! Are ref tests possible? Would be good to test rule painting with mixed writing modes.

> Source/WebCore/rendering/RenderMultiColumnBlock.h:46
> +    RenderMultiColumnFlowThread* flowThread() const { return m_flowThread; }

Should it return a const RenderMultiColumnFlowThread*?

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:65
> +int RenderMultiColumnSet::columnGap() const

This doesn't need to return a LayoutUnit?

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:69
> +    return static_cast<int>(style()->columnGap());

Maybe use pixelSnap, or make the rounding mode more explicit?

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:72
> +LayoutRect RenderMultiColumnSet::columnRectAt( unsigned index) const

Space after (

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:74
> +    // Compute the appropriate rect based off our information.

Not sure that this comment adds anything.
Comment 3 Dave Hyatt 2012-08-21 11:15:34 PDT
Fixed in r126177.