[New Multicolumn] Column gap is computed incorrectly

[Dave Hyatt]

We need to go to the parent block to get the column gap, since the RenderMultiColumnSet doesn't hold that value. :)

[Dave Hyatt]

Created attachment 189346 [details]
Patch

[Dave Hyatt]

Created attachment 189347 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 189347 [details]
Patch

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

> Source/WebCore/rendering/RenderMultiColumnSet.cpp:105
> +        return parentBlock->style()->fontDescription().computedPixelSize(); // "1em" is recommended as the normal gap setting. Matches <p> margins.

The comment makes me think that computedPixelSize() returns a value equivalent to 1em, which is confusing.

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

I think this should use clampToInteger() like roundedIntPoint() etc do. Weird that it's a float.

[Dave Hyatt]

(In reply to comment #3)
> (From update of attachment 189347 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189347&action=review
> 
> > Source/WebCore/rendering/RenderMultiColumnSet.cpp:105
> > +        return parentBlock->style()->fontDescription().computedPixelSize(); // "1em" is recommended as the normal gap setting. Matches <p> margins.
> 
> The comment makes me think that computedPixelSize() returns a value equivalent to 1em, which is confusing.
> 
> > Source/WebCore/rendering/RenderMultiColumnSet.cpp:106
> > +    return static_cast<int>(parentBlock->style()->columnGap());
> 
> I think this should use clampToInteger() like roundedIntPoint() etc do. Weird that it's a float.

Let me try just removing the cast completely. Technically now that subpixel layout is turned on, there is no reason to force the gap to be an integer any longer.

[Dave Hyatt]

Fixed in r143484.