Last Comment Bug 446273 - Column rule does not always stretch the full height of the column box.
: Column rule does not always stretch the full height of the column box.
Status: VERIFIED FIXED
: verified1.9.1
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Michael Ventnor
:
Mentors:
Depends on:
Blocks: 271586
  Show dependency treegraph
 
Reported: 2008-07-20 03:57 PDT by philippe (part-time)
Modified: 2009-01-14 12:37 PST (History)
10 users (show)
roc: blocking1.9.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (requires Ahem) (865 bytes, text/html)
2008-07-20 03:57 PDT, philippe (part-time)
no flags Details
screenshot (gecko vs WebKit) (15.33 KB, image/png)
2008-07-20 03:59 PDT, philippe (part-time)
no flags Details
testcase 2 (283 bytes, text/html)
2008-07-20 17:17 PDT, j.j.
no flags Details
Patch (1.24 KB, patch)
2008-07-21 00:48 PDT, Michael Ventnor
no flags Details | Diff | Splinter Review
Patch 2 (1.63 KB, patch)
2008-07-21 03:00 PDT, Michael Ventnor
roc: review+
roc: superreview+
Details | Diff | Splinter Review
Patch 3 (3.57 KB, patch)
2008-07-21 03:16 PDT, Michael Ventnor
no flags Details | Diff | Splinter Review

Description philippe (part-time) 2008-07-20 03:57:05 PDT
Created attachment 330446 [details]
testcase (requires Ahem)

If a column isn't filled with content, the rule is not drawn at full height of the column. WebKit always draws the rules at full height of the column, whereas Gecko stops with the text.

http://www.w3.org/TR/css3-multicol/#column4 states
'The length of the column gaps and column rules is equal to the length of the columns'
But also:
'Column rules are only drawn between columns that have content in the normal flow'

This _could_ be interpreted to validate what Gecko does. However, WebKit's implementation is more intuitive.
Comment 1 philippe (part-time) 2008-07-20 03:59:03 PDT
Created attachment 330447 [details]
screenshot (gecko vs WebKit)

Gecko on top, bottom is WebKit latest build

Gecko/2008071918 Minefield/3.1a1pre
Comment 2 Bill Gianopoulos [:WG9s] 2008-07-20 11:23:19 PDT
WORKSFORME with:

Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1a1pre) Gecko/2008072004 Firefox/3.01a1pre ID:2008072004

My rendering matches what your screenshot shows for WebKit.

Comment 3 Bill Gianopoulos [:WG9s] 2008-07-20 11:24:36 PDT
(In reply to comment #2)
> WORKSFORME with:
> 
> Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1a1pre)
> Gecko/2008072004 Firefox/3.01a1pre ID:2008072004
> 
> My rendering matches what your screenshot shows for WebKit.
> 
Oops.  Please ignore above.  Sorry for bugspam.  My rendering matches what you show for gecko. 
Comment 4 Michael Ventnor 2008-07-20 15:14:25 PDT
Intentional. Roc said that he wanted the column rule to be the minimum height of each column frame, so the last column rule changes size with the last column frame.
Comment 5 j.j. 2008-07-20 17:17:29 PDT
Created attachment 330514 [details]
testcase 2

IMHO this looks obviously wrong
Comment 6 philippe (part-time) 2008-07-20 17:20:26 PDT
(In reply to comment #4)
> Intentional. Roc said that he wanted the column rule to be the minimum height
> of each column frame, so the last column rule changes size with the last column
> frame.
As I said in comment 0, the spec seems to allow it.

Hmm, then we have an inter-operability problem with WebKit. I don't think there are other implementations of the multi-col module out there – or does PrinceXML implement it ?

I'll raise the issue with the CSS-WG.
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-07-20 20:31:51 PDT
(In reply to comment #4)
> Intentional. Roc said that he wanted the column rule to be the minimum height
> of each column frame, so the last column rule changes size with the last column
> frame.

Sorry, I think I was wrong. Let's just change it to match Webkit (and, I think, the spec).
Comment 8 Michael Ventnor 2008-07-21 00:48:17 PDT
Created attachment 330532 [details] [diff] [review]
Patch
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-07-21 02:13:43 PDT
This isn't right. The line should go all the way to the bottom of the column set and not depend on the content height of any particular column.
Comment 10 Michael Ventnor 2008-07-21 03:00:19 PDT
Created attachment 330540 [details] [diff] [review]
Patch 2
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-07-21 03:02:57 PDT
Comment on attachment 330540 [details] [diff] [review]
Patch 2

+  nsRect contentRect = GetContentRect();
+  contentRect -= GetRect().TopLeft();
+  contentRect += aPt;

Write this as one expression.

And of course you'll need tests. Make sure you test an element with padding so we know we're using the right edge.
Comment 12 Michael Ventnor 2008-07-21 03:16:56 PDT
Created attachment 330543 [details] [diff] [review]
Patch 3

With padding tests.
Comment 13 Dave Camp (:dcamp) 2008-07-21 23:36:30 PDT
Pushed to mozilla-central as revision 5c45945bd43b.
Comment 14 philippe (part-time) 2008-07-22 05:42:59 PDT
Roc, Michael: thank you !

verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008072202 Minefield/3.1a1pre

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