Last Comment Bug 404123 - [FIX]Percentage padding on <legend> triggers "ASSERTION: Bogus availSize.width. Should be bigger"
: [FIX]Percentage padding on <legend> triggers "ASSERTION: Bogus availSize.widt...
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9beta2
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on: 428423
Blocks: randomclasses reflow-refactor 354502
  Show dependency treegraph
 
Reported: 2007-11-16 19:16 PST by Jesse Ruderman
Modified: 2008-04-11 13:08 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (131 bytes, text/html)
2007-11-16 19:16 PST, Jesse Ruderman
no flags Details
Possible fix (9.28 KB, patch)
2007-11-29 19:58 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
dbaron: review+
dbaron: superreview+
Details | Diff | Review
Updated to comments (14.86 KB, patch)
2008-03-12 15:54 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
mbeltzner: approval1.9+
Details | Diff | Review

Description Jesse Ruderman 2007-11-16 19:16:42 PST
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
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-11-18 13:57:18 PST
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....
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-11-29 19:58:29 PST
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.
Comment 3 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-01-16 09:05:40 PST
See also bug 402850.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2008-02-15 14:32:47 PST
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)
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-02-15 14:48:10 PST
> 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.  :(
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-02-15 15:12:48 PST
David, see comment 5.
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-03-12 15:54:55 PDT
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.
Comment 8 Mike Beltzner [:beltzner, not reading bugmail] 2008-03-12 22:38:36 PDT
Comment on attachment 308982 [details] [diff] [review]
Updated to comments

a1.9=beltzner
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-03-14 13:48:36 PDT
Checked in.

Note You need to log in before you can comment on or make changes to this bug.