Column rule does not always stretch the full height of the column box.

VERIFIED FIXED

Status

()

Core
Layout
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: philippe (part-time), Assigned: Michael Ventnor)

Tracking

({verified1.9.1})

Trunk
verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
Created attachment 330447 [details]
screenshot (gecko vs WebKit)

Gecko on top, bottom is WebKit latest build

Gecko/2008071918 Minefield/3.1a1pre
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.

(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. 
(Assignee)

Comment 4

9 years ago
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

9 years ago
Created attachment 330514 [details]
testcase 2

IMHO this looks obviously wrong
(Reporter)

Comment 6

9 years ago
(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.
(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).
Flags: blocking1.9.1?
(Assignee)

Comment 8

9 years ago
Created attachment 330532 [details] [diff] [review]
Patch
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #330532 - Flags: superreview?(roc)
Attachment #330532 - Flags: review?(roc)
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.
(Assignee)

Comment 10

9 years ago
Created attachment 330540 [details] [diff] [review]
Patch 2
Attachment #330532 - Attachment is obsolete: true
Attachment #330540 - Flags: superreview?(roc)
Attachment #330540 - Flags: review?(roc)
Attachment #330532 - Flags: superreview?(roc)
Attachment #330532 - Flags: review?(roc)
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.
Attachment #330540 - Flags: superreview?(roc)
Attachment #330540 - Flags: superreview+
Attachment #330540 - Flags: review?(roc)
Attachment #330540 - Flags: review+
(Assignee)

Comment 12

9 years ago
Created attachment 330543 [details] [diff] [review]
Patch 3

With padding tests.
Attachment #330540 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Keywords: checkin-needed

Comment 13

9 years ago
Pushed to mozilla-central as revision 5c45945bd43b.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Reporter)

Comment 14

9 years ago
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
Status: RESOLVED → VERIFIED
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1

Updated

9 years ago
Keywords: verified1.9.1
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.