|Summary:||WebKit ignores column-rules wider than column-gap|
|Product:||WebKit||Reporter:||Eric Seidel (no email) <eric>|
|Component:||CSS||Assignee:||Morten Stenshorne <mstensho>|
|Severity:||Normal||CC:||anssi.kostiainen, benjamin, eoconnor, fishd, hyatt, koivisto, miket, morrita, odinho, ojan.autocc, simon.fraser, syoichi, vivekg, webkit.org, webkit.review.bot|
|Version:||523.x (Safari 3)|
|OS:||OS X 10.4|
Description Eric Seidel (no email) 2007-10-18 01:20:36 PDT
WebKit ignores column-rules wider than 20px See test case. There should be 30px wide column rules between each column.
Comment 1 Eric Seidel (no email) 2007-10-18 01:20:53 PDT
Created attachment 16718 [details] Test case
Comment 2 Eric Seidel (no email) 2007-10-18 01:22:43 PDT
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 Dave Hyatt 2007-10-18 01:40:11 PDT
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 Dave Hyatt 2007-10-18 01:44:49 PDT
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 6 Hajime Morrita 2013-02-13 01:11:55 PST
Tempted just to r+ but...
Comment 7 Benjamin Poulain 2013-02-13 01:54:53 PST
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.
Comment 8 Eric Seidel (no email) 2013-02-13 10:18:05 PST
Comment on attachment 188026 [details] Patch I assume this matches moz behavior?
Comment 9 Eric Seidel (no email) 2013-02-13 11:19:20 PST
Comment on attachment 188026 [details] Patch I'm entertained that I filed this bug.
Comment 10 Morten Stenshorne 2013-02-13 12:10:02 PST
Thanks! Yes, it matches both Mozilla and Opera... ehrm... Presto. :)
Comment 11 WebKit Review Bot 2013-02-13 12:13:22 PST
Comment on attachment 188026 [details] Patch Clearing flags on attachment: 188026 Committed r142770: <http://trac.webkit.org/changeset/142770>
Comment 12 WebKit Review Bot 2013-02-13 12:13:27 PST
All reviewed patches have been landed. Closing bug.
Comment 13 Dave Hyatt 2013-02-13 22:41:13 PST
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.
Comment 14 Morten Stenshorne 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 Eric Seidel (no email) 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 Morten Stenshorne 2013-02-14 13:52:19 PST
Reopening to add a fix for the new multicol code as well.
Comment 18 Eric Seidel (no email) 2013-02-15 08:50:52 PST
I'm confused. Is this new change tested?
Comment 19 Morten Stenshorne 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 Dave Hyatt 2013-02-19 22:19:49 PST
I went ahead and handled this.