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)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

Details

(Keywords: crash, testcase, Whiteboard: [xul frame construction])

Crash Data

Attachments

(2 files)

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.
Attached file Testcase
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
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
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]
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...
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...
I agree that both are bogus.
grid { display: -moz-grid; } in xul.css takes care of 1) anyway.
Attached patch PatchSplinter Review
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 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+
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: nobody → bzbarsky
Fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
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.
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
Crash Signature: [@ nsFrameItems::AddChild]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: