Closed
Bug 408904
Opened 17 years ago
Closed 17 years ago
Crash [@ nsSprocketLayout::PopulateBoxSizes] with <xul:grid>
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: neil)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(2 files)
166 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
667 bytes,
patch
|
enndeakin
:
review+
bzbarsky
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
Loading the testcase crashes Firefox [@ nsSprocketLayout::PopulateBoxSizes].
Updated•17 years ago
|
Flags: blocking1.9?
![]() |
||
Comment 1•17 years ago
|
||
Looks like a regression from bug 384874. The code sets |last = nsnull|, then enters the loop. aBoxSizes is not null, but all the things in that list are bogus, so we skip over all of them. So currentBox ends up null. So now we have |!currentBox && aBoxSizes|, so at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsSprocketLayout.cpp&rev=1.62&mark=815,818,822#815 we end up with a guaranteed crash (since |last| is still null). We need to either null-check |last| here or set |last| when skipping over bogus boxes or something. Not sure what the right thing is, esp. since we reuse aBoxSizes later so it needs to have all the box sizes in it or something.
Blocks: 384874
Keywords: regression
Assignee | ||
Comment 2•17 years ago
|
||
Yeah, we always need to set last. See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsSprocketLayout.cpp&rev=1.62&mark=896-897#895 I suppose the alternate fix is wrap the block (except those lines) inside if (!currentBox || !currentBox->bogus) { }
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #293821 -
Flags: superreview?(bzbarsky)
Attachment #293821 -
Flags: review?(enndeakin)
Updated•17 years ago
|
Attachment #293821 -
Flags: review?(enndeakin) → review+
![]() |
||
Comment 3•17 years ago
|
||
Comment on attachment 293821 [details] [diff] [review] Proposed patch Looks good.
Attachment #293821 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 293821 [details] [diff] [review] Proposed patch Crash fix.
Attachment #293821 -
Flags: approval1.9?
Comment 5•17 years ago
|
||
Comment on attachment 293821 [details] [diff] [review] Proposed patch a=beltzner for 1.9
Attachment #293821 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 6•17 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
![]() |
||
Updated•17 years ago
|
Flags: in-testsuite?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Updated•13 years ago
|
Crash Signature: [@ nsSprocketLayout::PopulateBoxSizes]
You need to log in
before you can comment on or make changes to this bug.
Description
•