The default bug view has changed. See this FAQ.

[FIX]Percentage padding on <legend> triggers "ASSERTION: Bogus availSize.width. Should be bigger"

RESOLVED FIXED in mozilla1.9beta2

Status

()

Core
Layout: Form Controls
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla1.9beta2
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
Created attachment 289105 [details]
testcase

Loading the testcase gives me:

###!!! ASSERTION: Bogus availSize.width.  Should be bigger: 'availSize.width >= mLegendRect.width', file /Users/jruderman/trunk/mozilla/layout/forms/nsFieldSetFrame.cpp, line 533
Yeah, the fieldset code I wrote on reflow branch really doesn't handle percentage stuff all that well.  And this particular assertion is probably just wrong....
Blocks: 300030
Created attachment 290819 [details] [diff] [review]
Possible fix

This removes the unenforceable assertion, as well as a lot of the complexity, and the bogus possible manipulation of NS_UNCONSTRAINEDSIZE.

I'm also open to removing some of those !important rules this patch is adding, if we want to.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #290819 - Flags: superreview?(dbaron)
Attachment #290819 - Flags: review?(dbaron)
Summary: Percentage padding on <legend> triggers "ASSERTION: Bogus availSize.width. Should be bigger" → [FIX]Percentage padding on <legend> triggers "ASSERTION: Bogus availSize.width. Should be bigger"
OS: Mac OS X → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9 M10
Blocks: 354502
Blocks: 364745
No longer blocks: 364745
See also bug 402850.
Comment on attachment 290819 [details] [diff] [review]
Possible fix

In the forms.css changes, I think you should also add rules for
min-height and max-height.

Also, to be compatible with what you were doing before, I think you need 
to specify width:-moz-max-content, not width:auto.  Otherwise the
min-width computation for the fieldset will assume line breaking inside  
the legend.  Or was that change intentional?
(If you do this, fix the assertion in nsFieldSetFrame::Reflow to match.)

(Could you at least add a testcase for min-width computation of a
fieldset with a legend that has multiple words in it?)

r+sr=dbaron with that (although it's also ok if your change to allow the contents of the legend to wrap was intentional -- assuming you've tested that it works reasonably)
Attachment #290819 - Flags: superreview?(dbaron)
Attachment #290819 - Flags: superreview+
Attachment #290819 - Flags: review?(dbaron)
Attachment #290819 - Flags: review+
> I think you should also add rules for min-height and max-height.

Will do.

> Also, to be compatible with what you were doing before, I think you need 
> to specify width:-moz-max-content, not width:auto.  Otherwise the
> min-width computation for the fieldset will assume line breaking inside  
> the legend.  Or was that change intentional?

This was intentional.  The white-space:nowrap should keep the current behavior for normal cases, but if someone wants their legends to wrap, they can.  That fixes bug 354502.

> (Could you at least add a testcase for min-width computation of a
> fieldset with a legend that has multiple words in it?)

Yeah, I need to write some tests.  I did test this stuff at the time, but didn't seem to get those data: URIs into reftest format.  :(
David, see comment 5.
Created attachment 308982 [details] [diff] [review]
Updated to comments

Drivers, this is a patch that makes sure that we don't do unconstrained math with fieldsets...  Risk should be low-ish.
Attachment #308982 - Flags: approval1.9?
Comment on attachment 308982 [details] [diff] [review]
Updated to comments

a1.9=beltzner
Attachment #308982 - Flags: approval1.9? → approval1.9+
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 428423
You need to log in before you can comment on or make changes to this bug.