Bug 15553 - WebKit ignores column-rules wider than column-gap
: WebKit ignores column-rules wider than column-gap
Status: RESOLVED FIXED
: WebKit
CSS
: 523.x (Safari 3)
: Macintosh Mac OS X 10.4
: P2 Normal
Assigned To:
: http://www.w3.org/TR/css3-multicol/#c...
:
:
:
  Show dependency treegraph
 
Reported: 2007-10-18 01:20 PST by
Modified: 2013-02-19 22:19 PST (History)


Attachments
Test case (1.44 KB, text/html)
2007-10-18 01:20 PST, Eric Seidel
no flags Details
Patch (4.02 KB, patch)
2013-02-13 00:01 PST, Morten Stenshorne
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.71 KB, patch)
2013-02-14 14:04 PST, Morten Stenshorne
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2007-10-18 01:20:36 PST
WebKit ignores column-rules wider than 20px

See test case.  There should be 30px wide column rules between each column.
------- Comment #1 From 2007-10-18 01:20:53 PST -------
Created an attachment (id=16718) [details]
Test case
------- Comment #2 From 2007-10-18 01:22:43 PST -------
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."
------- Comment #3 From 2007-10-18 01:40:11 PST -------
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.
------- Comment #4 From 2007-10-18 01:44:49 PST -------
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.
------- Comment #5 From 2013-02-13 00:01:06 PST -------
Created an attachment (id=188026) [details]
Patch
------- Comment #6 From 2013-02-13 01:11:55 PST -------
Tempted just to r+ but...
------- Comment #7 From 2013-02-13 01:54:53 PST -------
(From update of attachment 188026 [details])
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.
------- Comment #8 From 2013-02-13 10:18:05 PST -------
(From update of attachment 188026 [details])
I assume this matches moz behavior?
------- Comment #9 From 2013-02-13 11:19:20 PST -------
(From update of attachment 188026 [details])
I'm entertained that I filed this bug.
------- Comment #10 From 2013-02-13 12:10:02 PST -------
Thanks! Yes, it matches both Mozilla and Opera... ehrm... Presto. :)
------- Comment #11 From 2013-02-13 12:13:22 PST -------
(From update of attachment 188026 [details])
Clearing flags on attachment: 188026

Committed r142770: <http://trac.webkit.org/changeset/142770>
------- Comment #12 From 2013-02-13 12:13:27 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #13 From 2013-02-13 22:41:13 PST -------
(From update of attachment 188026 [details])
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.
------- Comment #14 From 2013-02-13 23:20:27 PST -------
I can do it. Do I have to open a new bug, or can I use this one again?
------- Comment #15 From 2013-02-13 23:59:26 PST -------
Either way.  Generally folks do one bug per change (because the tools assume that), but up to you. :)
------- Comment #16 From 2013-02-14 13:52:19 PST -------
Reopening to add a fix for the new multicol code as well.
------- Comment #17 From 2013-02-14 14:04:44 PST -------
Created an attachment (id=188420) [details]
Patch
------- Comment #18 From 2013-02-15 08:50:52 PST -------
I'm confused. Is this new change tested?
------- Comment #19 From 2013-02-17 05:27:56 PST -------
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?
------- Comment #20 From 2013-02-19 22:19:49 PST -------
I went ahead and handled this.