Closed Bug 446273 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: phiw2, Assigned: ventnor.bugzilla)

References

Details

(Keywords: verified1.9.1)

Attachments

(4 files, 2 obsolete files)

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.
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. 
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.
Attached file testcase 2
IMHO this looks obviously wrong
(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).
Attached patch Patch (obsolete) — Splinter Review
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.
Attached patch Patch 2 (obsolete) — Splinter Review
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+
Attached patch Patch 3Splinter Review
With padding tests.
Attachment #330540 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed to mozilla-central as revision 5c45945bd43b.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
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
Keywords: verified1.9.1
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.