Bug 15553 – WebKit ignores column-rules wider than column-gap

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Eric Seidel (no email)]

WebKit ignores column-rules wider than 20px

See test case.  There should be 30px wide column rules between each column.

[Eric Seidel (no email)]

Created attachment 16718 [details]
Test case

[Eric Seidel (no email)]

Actually, we're only ignoring column rules when they are larger than the column-gap.  This is in disagreement with the spec:

http://www.w3.org/TR/css3-multicol/#column4

"Column rules do not take up space. That is, the presence or thickness of a column rule will not alter the placement of anything else. If a column rule is wider than its gap, the column rule will overlap adjacent column boxes."

[Dave Hyatt]

The rule is wider than the gap.... unless the spec is changed, I chose not to paint the rule in that case since it couldn't fit.

[Dave Hyatt]

I guess it changed.  What is implemented may not match the spec, since the spec changes so much regularly.  This is one reason I quit hacking on this stuff, to give it a chance to settle down.

[Morten Stenshorne]

Created attachment 188026 [details]
Patch

[Hajime Morrita]

Tempted just to r+ but...

[Benjamin Poulain]

Comment on attachment 188026 [details]
Patch

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

> LayoutTests/ChangeLog:7
> +        Paint column rules even if they are wider than the gap.
> +        Rules wider than the gap should just overlap with column contents.

For next time: you don't have to copy the description in each ChangeLog :)

Unless the test needs some explanation, you can usually leave the LayoutTest's ChangeLog without description.

[Eric Seidel (no email)]

Comment on attachment 188026 [details]
Patch

I assume this matches moz behavior?

[Eric Seidel (no email)]

Comment on attachment 188026 [details]
Patch

I'm entertained that I filed this bug.

[Morten Stenshorne]

Thanks! Yes, it matches both Mozilla and Opera... ehrm... Presto. :)

[WebKit Review Bot]

Comment on attachment 188026 [details]
Patch

Clearing flags on attachment: 188026

Committed r142770: <http://trac.webkit.org/changeset/142770>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dave Hyatt]

Comment on attachment 188026 [details]
Patch

You missed patching RenderMultiColumnSet (the new multi-column implementation that is being worked on. In general you need to make sure to see if bugs you fix in the old multi-column code are present in the new code and make sure to patch both where applicable. I can either fix it myself or if you'd like to submit a second patch that fixes RenderMultiColumnSet as well, that would be appreciated.

[Morten Stenshorne]

I can do it. Do I have to open a new bug, or can I use this one again?

[Eric Seidel (no email)]

Either way.  Generally folks do one bug per change (because the tools assume that), but up to you. :)

[Morten Stenshorne]

Reopening to add a fix for the new multicol code as well.

[Morten Stenshorne]

Created attachment 188420 [details]
Patch

[Eric Seidel (no email)]

I'm confused. Is this new change tested?

[Morten Stenshorne]

Yes, I've tested it (rules are now painted even if they are wider than the gap), but the new region-based multicol code has other problems which prevent our current testcases from passing, even with this bug fixed. I wasn't sure if it's worth it to write special tests (either attached to this bug or under LayoutTests/fast/multicol/) for the new multicol code, but should I?

[Dave Hyatt]

I went ahead and handled this.