277355 - Column layout mishandles margins on child divs [columns]

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: 2005-01-06-08 trunk build

STEPS TO REPRODUCE:

1) Load attached testcase
2) Note that text overlaps when it should not

NOTES: It basically looks like the right margin is being ignored.  A little
experimentation shows that this is indeed what is happening -- the right margin
is ignored (eg setting the left margin to 0 makes the text fill the column,
instead of just the first 14em of the column.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

So one problem, based on printfs I stuck in, is that when we go to reflow the
child and create its reflow state, we end up calling into Init() with
aContainingBlockWidth == -1.  So this code calls
ComputeContainingBlockRectangle, which uses aContainingBlockRS->mComputedWidth
as the aContainingBlockWidth.  In this case, that happens to be
NS_SHRINKWRAPWIDTH (why?).  So when we're in ComputeBlockBoxData() we end up
ignoring the availableWidth, since aContainingBlockWidth is shrink-wrap, and set
mComputedWidth to NS_UNCONSTRAINEDSIZE.  So it looks like the column-content
block is screwing up somehow, probably by not doing a constrained reflow
correctly after it does a size-finding reflow....  But why is it doing a
size-finding reflow if it knows the column width?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

One issue is that nsColumnSetFrame needs to pass in a 'fake' containing block
width so we get the right CB width.

A bigger issue was that -moz-column-content frames were getting
'display:inline' (initial value) which was causing confusion since we always
create block frames for them. They should have display:inherit (in case we ever
want to allow tables to be columnized, for example).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 172313 [details] [diff] [review]
fix

Hmm....  So why is this the right thing to do?	This will keep the reflow state
from computing a height at all; is that what we want?  That is should
percentage heights in columns work?  I don't think they will, with this
patch...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 172313 [details] [diff] [review]
fix

r- per comment 4.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: testcase

appropriate testcase

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

OK, so this passes the correct height into the reflow state and also whacks
ua.css so that the column block carries it down to any %-height children. It
fixes the testcase.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173028 [details] [diff] [review]
fix

r+sr=bzbarsky.	Guess we're saved from the quirks percent-height handling by
the column-set frame... ;)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 10

[Boris Zbarsky [:bzbarsky]]

Though I bet there is one case this still misses: height:inherit on the child....

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, for that we need to mess around with the style context tree like we do for
scrollframes.