Closed Bug 329768 Opened 19 years ago Closed 19 years ago

Crash [@ nsTableOuterFrame::IsAutoWidth] setting display:table on html and using position:absolute, display:block in object

Categories

(Core :: Layout, defect)

x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: bernd_mozilla)

References

Details

(5 keywords)

Crash Data

Attachments

(3 files, 1 obsolete file)

See upcoming testcase, which crashes on load. This regressed between 2005-09-21 and 2005-09-22, a regression from bug 1156, I guess.
Attached file testcase
Talkback ID: TB16089457H nsTableOuterFrame::IsAutoWidth nsTableOuterFrame::Reflow nsRuleNode::GetParentData
It doesn't crash all the time (but most of the time). If it doesn't crash, then hit shift->reload.
0x0146dace in nsIFrame::GetStyleData (this=0x0, aSID=eStyleStruct_Position) at /home/chb/mozilla/layout/svg/base/src/../../../generic/nsIFrame.h:579 579 NS_ASSERTION(mStyleContext, "No style context found!"); this is NULL... looks like it's because of this: (gdb) frame 3 #3 0x016c5e11 in nsTableOuterFrame::Reflow (this=0x9b7c484, aPresContext=0x9b91110, aDesiredSize=@0xbfd4cac8, aOuterRS=@0xbfd4ca10, aStatus=@0xbfd4ccd8) at ../../../../mozilla/layout/tables/nsTableOuterFrame.cpp:1871 1871 IsAutoWidth(*mInnerTableFrame, &isPctWidth); (gdb) print mInnerTableFrame $1 = (class nsTableFrame *) 0x0
OS: Windows XP → All
Attached file without col
Summary: Crash setting display:table on html and using position:aboslute, display:table-column in object → Crash setting display:table on html and using position:aboslute, display:block in object
Summary: Crash setting display:table on html and using position:aboslute, display:block in object → Crash setting display:table on html and using position:absolute, display:block in object
So on that last testcase, when I crash (after a few reloads) the relevant part of the frame tree looks like: Canvas(html)(-1)@0x8661d70 [view=0x865e4e8] {0,0,13944,9030} [state=00042001] [content=0x863f418] [sc=0x8663934] pst=:-moz-scrolled-canvas< TableOuter(html)(-1)@0x866488c {0,0,0,0} [state=00050403] [content=0x863f418] [sc=0x8663d08] pst=:-moz-table-outer< Area(script)(1)@0x864bf24 [view=0x85f2850] {0,0,0,0} [state=00c02502] sc=0x864be78(i=1,b=0)< line 0x8663e10: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4001] {0,0,0,0} < Text(0)@0x8664340[0,0,F] {0,0,0,0} [state=00000422] sc=0x864bfb0 pst=:-moz-non-element< "" > > > > > which is Just Wrong...
OK, so this only happens if we restyle <html> before doing our pending frame reconstruct on the <object>.... Right after we restyle the <html> we have the following frame tree: Canvas(html)(-1)@0x89a6e28 [view=0x89056f0] {0,0,13944,9030} [state=00042000] [content=0x89a6ae8] [sc=0x89c0c94] pst=:-moz-scrolled-canvas< TableOuter(html)(-1)@0x899f254 {0,0,0,0} [state=00050402] [content=0x89a6ae8] [sc=0x899e3c0] pst=:-moz-table-outer< .... Block(html)(-1)@0x89c0f54 {0,0,0,0} [state=00c40402] sc=0x89c039c(i=0,b=1) pst=:-moz-cell-content< line 0x899f350: count=1 state=block,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4009] {0,0,0,0} < Block(body)(1)@0x899e460 {0,0,0,0} [state=00040402] sc=0x89c0238(i=1,b=0)< .... Absolute-list< Area(script)(1)@0x89a20a0 [view=0x89b1c58] {0,0,0,0} [state=00c02502] sc=0x89a1ff4(i=1,b=0)< line 0x899e4c8: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4001] {0,0,0,0} < Text(0)@0x89a217c[0,0,F] {0,0,0,0} [state=00000422] sc=0x89a212c pst=:-moz-non-element< "" > > > > which is also wrong, imho.
OK, so the setup in comment 6 comes about because in ConstructBlock we push the block as an abs pos containing block if there isn't one pushed already. That's not the real bug here. The real bug (or the first half) is that GetAbsoluteContainingBlock does something different -- it returns mInitialContainingBlock if it finds nothing else. I was _sure_ we had a bug on that... In any case, what happens here is: 1) We reframe the <html> because of the style change.yet. 2) Right after that (before we get a chance to reflow) we reframe the <object>. 3) We start constructing frames and call GetAbsoluteContainingBlock() to get one. This returns mInitialContainingBlock, which is the outer table frame(!). 4) We construct a frame for the <script>. It's an abs pos frame, so we put it on the abs pos list of our absolute containing block. But that list is empty and the outer table has had no initial reflow yet, so we use SetInitialChildList to do that. 5) nsTableFrame::SetInitialChildList assumes that any aListName that is not captionList is null and sticks stuff in the principal list, clobbering whatever is there. It doesn't bother to assert that aListName is null or that mFrames is empty... If it did, debugging this would have taken about 30 seconds. :( So as I see it we have two+ bugs: 1) GetAbsoluteContainingBlock() is broken. 2a) nsTableOuterFrame::SetInitialChildList is kinda broken. 2b) nsTableOuterFrame::Reflow doesn't deal with a null mInnerTableFrame; should it?
Flags: blocking1.9a1+
2a) nsTableOuterFrame::SetInitialChildList is kinda broken. I will fix this. 2b) nsTableOuterFrame::Reflow doesn't deal with a null mInnerTableFrame; should it? I thought that the code http://lxr.mozilla.org/mozilla/source/layout/tables/nsTableOuterFrame.cpp#1913 and http://lxr.mozilla.org/mozilla/source/layout/tables/nsTableOuterFrame.cpp#1330 does do exactly that (bug 189751), but its probably much better to move this just up to the entrance of the method. Please note that under normal conditions we always create a inner table frame even if we create a outer pseudo for captions.
Attached patch patch for 2a and 2b (obsolete) — Splinter Review
Attachment #214759 - Flags: superreview?(bzbarsky)
Attachment #214759 - Flags: review?(bzbarsky)
Summary: Crash setting display:table on html and using position:absolute, display:block in object → Crash [@ nsTableOuterFrame::IsAutoWidth] setting display:table on html and using position:absolute, display:block in object
Comment on attachment 214759 [details] [diff] [review] patch for 2a and 2b >Index: nsTableOuterFrame.cpp >+ NS_ASSERTION(PR_FALSE, "incomplete children"); NS_ERROR please? r+sr=bzbarsky. If that fixes the crash, we should file a followup bug on GetAbsoluteContainingBlock being broken, assuming one doesn't exist already.
Attachment #214759 - Flags: superreview?(bzbarsky)
Attachment #214759 - Flags: superreview+
Attachment #214759 - Flags: review?(bzbarsky)
Attachment #214759 - Flags: review+
Assignee: nobody → bernd_mozilla
Attachment #214759 - Attachment is obsolete: true
Attachment #215476 - Flags: approval1.8.0.3?
Attachment #215476 - Flags: approval-branch-1.8.1?(roc)
The patch reduces a crash risk by checking more aggressively than before.
fixed on trunk
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
I filed the followup bug 330909 as requested.
*** Bug 330499 has been marked as a duplicate of this bug. ***
Verified FIXED using SeaMonkey 1.5a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060319 SeaMonkey/1.5a on: https://bugzilla.mozilla.org/attachment.cgi?id=214426 --and-- https://bugzilla.mozilla.org/attachment.cgi?id=214530 testcases: no crash.
Status: RESOLVED → VERIFIED
Attachment #215476 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
Comment on attachment 215476 [details] [diff] [review] patch that got checked in Please check in promptly on the 1.8.0 branch (and the 1.8.1 branch too). Thanks!
Attachment #215476 - Flags: approval1.8.0.3? → approval1.8.0.3+
fixed on the 1.8.0 and 1.8 branch
v.fixed on 1.8.0 branch: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4, no crash with either testcase.
Blocks: 340946
Crash Signature: [@ nsTableOuterFrame::IsAutoWidth]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: