188787 - Incremental reflow problem when setting label

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Setting .label or the label attribute on a XUL checkbox will cause the text in
it to wrap to as narrow a width as possible, if it's in the same hbox as a
spacer.  Having the label be defined statically (via attribute) does _not_ lead
to such wrapping.

Testcase coming up; this is blocking making the helper app dialog not suck.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: simpler testcase

This may be easier to debug; not a problem with <radio> per se....

Comment 3

[Boris Zbarsky [:bzbarsky]]

So... this makes "sense".  As far as I can tell (and this could be deeply wrong;
I'm still wrapping my brain around the code), here is what's up.  We have a box
that's basically 0 by 0 (0 by 238 twips, actually, if my printfs are not lying
to me).  This box is actually a nsBoxToBlockAdaptor that contains the <label> block.

Now we trigger a reflow.  The nsBoxToBlockAdaptor is first flown at its current
width/height.  It constructs an nsHTMLReflowState with the available sizes being
the current width (0) and NS_INTRINSICSIZE  It sets the computed width/height to
the current width/height (0, 238) and flows its block.  The block comes back
with a desired width that's as small as possible and a corresponding desired
height (it just wraps the text as much as it can).

I tried setting the computed width to NS_INTRINSICSIZE and that "fixed" this bug
(it broke sizing of all sorts of other things, however).

It seems like we should flow the block at both current size and intrinsic size
and take the max of the two, or something... still trying to understand what
nsSprocketLayout is doing with this stuff and whether it can be improved without
breaking other things...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: hack upon hack, is the adaptor

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 111606 [details] [diff] [review]
hack upon hack, is the adaptor

This seems like a major hack to me.  Then again, so does 90% of the code in
that method....

I've been unable to make this do anything particularly incorrect, in my testing
so far.  Could someone please tell my why we should not be doing this (other
than the obvious cost of an extra reflow)?

Oh, and I suppose we could set mComputeMEW ourselves on the desired size (we'd
need to instantiate our own).  I've not managed to create a case where
aDesiredSize.mComputeMEW is _not_ set....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 111606 [details] [diff] [review]
hack upon hack, is the adaptor

This breaks the profile manager. :(

Comment 7

[[e-mail]]

bz: how do tables cope? If I open Composer, insert an unconstrained table, and
start typing text into it, it automagically resizes until the desired width of
the table exceeds the available width, at which point the text starts wrapping.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I'm not sure I understand quite how tables cope yet.. there is the secondary
problem that this algorithm we're talking about here is a bridge between two
different box models, so it has to deal with some crap that tables don't need to
handle....

Comment 9

[[e-mail]]

Is there an easy way to find the "slack" space available and add that on first?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Again, not sure.  I'm still figuring out how XUL reflow works....

Comment 11

[[e-mail]]

Fixed by the patch in bug 255270.