Closed
Bug 388374
Opened 17 years ago
Closed 17 years ago
[FIXr]"ASSERTION: How did our kid's height change if nothing was dirty?" with tables, padding, <select>
Categories
(Core :: Layout: Form Controls, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files)
|
(deleted),
application/xhtml+xml
|
Details | |
|
(deleted),
text/html
|
Details | |
|
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
dbaron
:
approval1.9+
|
Details | Diff | Splinter Review |
Loading the testcase triggers:
###!!! ASSERTION: How did our kid's height change if nothing was dirty?: 'oldVisibleHeight == GetScrolledFrame()->GetSize().height', file /Users/jruderman/trunk/mozilla/layout/forms/nsListControlFrame.cpp, line 665
The testcase is fragile: changing the amount of text in the testcase affects whether the assertion is triggered :(
|
Assignee | |
Comment 1•17 years ago
|
||
Might depend on the exact fonts too... I can't reproduce. :(
|
Reporter | |
Comment 2•17 years ago
|
||
Can you reproduce with this?
|
|
Assignee | |
Comment 3•17 years ago
|
||
Indeed, I can.
David, the issue here is just that heights do depend on widths, so if the select width changed we should be prepared for the heights of the options to change. In this case that's due to percentage vertical padding, but allowing options to wrap would also have this effect, I bet. It's really a silly thing to have overlooked...
Attachment #273034 -
Flags: superreview?(dbaron)
Attachment #273034 -
Flags: review?(dbaron)
|
|
Assignee | |
Updated•17 years ago
|
Assignee: nobody → bzbarsky
Flags: in-testsuite?
Priority: -- → P3
Summary: "ASSERTION: How did our kid's height change if nothing was dirty?" with tables, padding, <select> → [FIX]"ASSERTION: How did our kid's height change if nothing was dirty?" with tables, padding, <select>
Target Milestone: --- → mozilla1.9beta1
|
||
Comment 4•17 years ago
|
||
Comment on attachment 273034 [details] [diff] [review]
Proposed fix
r+sr=dbaron. (Seems like checking ShouldReflowAllKids() rather than just mHResize might be useful for some vertical resize cases, although I can't actually think of any that can happen when height is auto.)
Attachment #273034 -
Flags: superreview?(dbaron)
Attachment #273034 -
Flags: superreview+
Attachment #273034 -
Flags: review?(dbaron)
Attachment #273034 -
Flags: review+
|
|
Assignee | |
Updated•17 years ago
|
Summary: [FIX]"ASSERTION: How did our kid's height change if nothing was dirty?" with tables, padding, <select> → [FIXr]"ASSERTION: How did our kid's height change if nothing was dirty?" with tables, padding, <select>
|
|
Assignee | |
Comment 5•17 years ago
|
||
Comment on attachment 273034 [details] [diff] [review]
Proposed fix
Make the optimization not be so optimized that it optimizes away needed reflows. No correctness risk, and I doubt this will affect performance much, if at all.
Attachment #273034 -
Flags: approval1.9?
|
|
||
Comment 6•17 years ago
|
||
Comment on attachment 273034 [details] [diff] [review]
Proposed fix
a19=dbaron
Attachment #273034 -
Flags: approval1.9? → approval1.9+
|
|
Assignee | |
Comment 7•17 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•