Closed
Bug 283140
Opened 19 years ago
Closed 19 years ago
Crash if display:-moz-grid has XBL base tag [@ nsFrameItems::AddChild]
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
VERIFIED
FIXED
mozilla1.8beta2
People
(Reporter: martijn.martijn, Assigned: bzbarsky)
Details
(Keywords: crash, testcase, Whiteboard: [xul frame construction])
Crash Data
Attachments
(2 files)
388 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
1.36 KB,
patch
|
neil
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
The upcoming testcase crashes Mozilla when hovering over the textbox. See Talkback ID: TB3857133X nsFrameItems::AddChild [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 724] nsCSSFrameConstructor::ConstructFrameByDisplayType [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 6625] nsCSSFrameConstructor::ConstructFrameInternal [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 7586] nsCSSFrameConstructor::ConstructFrameInternal [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 7482] nsCSSFrameConstructor::ConstructFrame [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 7417] nsCSSFrameConstructor::ContentInserted [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9173] nsCSSFrameConstructor::RecreateFramesForContent [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11536] nsCSSFrameConstructor::RestyleElement [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10130] nsCSSFrameConstructor::ProcessOneRestyle [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13484] nsCSSFrameConstructor::ProcessPendingRestyles [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13526] nsCSSFrameConstructor::RestyleEvent::HandleEvent [c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13578] 0x778b0c24 Bug 281333 has the same stack, it seems, so it might be related, I guess.
Reporter | ||
Comment 1•19 years ago
|
||
Reporter | ||
Comment 2•19 years ago
|
||
Doesn't crash for me with: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/2004102604 Crashes for me with: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/2004102705 http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-10-26+04%3A00%3A00&maxdate=2004-10-27+06%3A00%3A00&cvsroot=%2Fcvsroot
Assignee | ||
Comment 3•19 years ago
|
||
So this is just a bug in XUL frame construction, I think. For "grid" and "inline-grid" display values, it only constructs the frame if the tag is "grid" or there is no XBL base tag (and for <textbox> the XBL base tag is <xul:box>). So ConstructXULFrame constructs nothing in this case. Then we assert on an unknown display type in ConstructFrameByDisplayType and crash. Is there a reason we don't construct a grid frame when we have an XBL base tag? Note that this just flat-out crashes if style="display: -moz-grid" is added to the textbox (no hover required).
OS: Windows 2000 → All
Hardware: PC → All
Assignee | ||
Updated•19 years ago
|
Summary: Crash [@ nsFrameItems::AddChild] with testcase, using textbox:hover{display:-moz-grid} → Crash if display:-moz-grid has XBL base tag [@ nsFrameItems::AddChild]
Whiteboard: [xul frame construction]
Comment 4•19 years ago
|
||
It seems silly that you need the tag, you can easily make e.g. a radiogroup a grid row by styling it, so why not a grid too? That's assuming we've caught all the other issues resulting from incomplete grid parts, of course...
Assignee | ||
Comment 5•19 years ago
|
||
It's not needing the tag... The exact test for whether to construct a grid frame is: if ((!aXBLBaseTag && (display->mDisplay == NS_STYLE_DISPLAY_INLINE_GRID || display->mDisplay == NS_STYLE_DISPLAY_GRID)) || aTag == nsXULAtoms::grid) { So it will construct a grid if 1) The tag is <grid> or 2) The display is grid or inline-grid, and there is no xbl base tag. This last condition (no xbl base tag) is what causes the crash here, and it seems bogus to me... It also seems bogus to create a grid for a <grid> that's had its display changed, of course...
Comment 6•19 years ago
|
||
I agree that both are bogus. grid { display: -moz-grid; } in xul.css takes care of 1) anyway.
Assignee | ||
Comment 7•19 years ago
|
||
I think hyatt just forgot to change this code like he changed the box code when he added XUL display types.
Attachment #175315 -
Flags: superreview?(roc)
Attachment #175315 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 8•19 years ago
|
||
Comment on attachment 175315 [details] [diff] [review] Patch I'm sure the compiler can cope but it would look nicer as a switch ;-)
Attachment #175315 -
Flags: review?(neil.parkwaycc.co.uk) → review+
Assignee | ||
Comment 9•19 years ago
|
||
I'm not going to bother prettying up code that just needs to be removed.... (which is all the XUL display code -- it needs to move to a different method, etc).
Attachment #175315 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Updated•19 years ago
|
Assignee: nobody → bzbarsky
Assignee | ||
Comment 10•19 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
Comment 11•19 years ago
|
||
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050228 The testcase doesn´t crash anymore, but flickers fast, depending on cursor position only the cursor is changing, or the whole box is redrawn. I also can get it reduced to a zero width box, only 4 by 6 px because of margin or borders or so.
Assignee | ||
Comment 12•19 years ago
|
||
Yeah. That's all fine; the behavior for the markup in the testcase is pretty undefined. We just shouldn't crash; looking pretty is optional.
Verified FIXED using the testcase at https://bugzilla.mozilla.org/attachment.cgi?id=175120,using build 2005-03-03-05 on Windows XP Seamonkey trunk. Talkback data here really isn't useful in verification, because the last crash shown is from 2005022205. Anyhow, this is definitely fixed.
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ nsFrameItems::AddChild]
You need to log in
before you can comment on or make changes to this bug.
Description
•