Closed Bug 56563 Opened 24 years ago Closed 24 years ago

crash opening page

Categories

(Core :: Layout, defect, P1)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: buster, Assigned: karnaze)

References

()

Details

(Keywords: crash, Whiteboard: [rtm-][Fix in hand])

Attachments

(8 files)

spun off from bug 53935 janc@netscape.com reports a crash at this page on branch only, all platforms. Will attach talkback reports.
It's not branch-only; I crash with the same stack in a new win98 trunk build. The assertion, btw, is: ###!!! ASSERTION: can't find deleted frame in lines: 'nsnull != line', file C:\m ozilla\layout\html\base\src\nsBlockFrame.cpp, line 5461 should fix for RTM
Keywords: crash, rtm
Whiteboard: [rtm need info]
investigating...
Status: NEW → ASSIGNED
Priority: P3 → P1
Attached file minimized test case (deleted) —
karnaze and harish: Rick and I are working on this crasher. It looks like the crash is coming about because the content and/or frame model are corrupt at the time we do an incremental reflow. See the test case (it's hideous mark-up, but at least it's small.) Rick thinks the code that moves content out of an illegally-formed table isn't doing the right thing. Harish could look at that. Secondly, given the incorrect content model, I think Chris' code in nsCSSFrameConstructor might not be doing the right thing either. That should also get fixed, because this same sort of thing could have happened through the DOM (maybe...can the DOM create unknown tags?)
Here's the content model: docshell=01089E90 html@02861938 refcount=6< head@02861848 refcount=2< > Text@028A2C50 refcount=3<\n> body@028A30B8 refcount=3< table@028D1A18 refcount=10< Text@028D2820 refcount=2<\n> sh@028D3928 _moz-userdefined= refcount=5< sh@028D4A48 _moz-userdefined= refcount=5< Text@028D4920 refcount=3<\n> p@028D5B68 refcount=3< Text@028D5A90 refcount=3<steve\n> > p@028D5968 refcount=3< Text@028D5890 refcount=3<\n> img@028D84FC src=http://www.inktomi.com/logo/PowByInkMed.gif height=40 width=126 refcount=3<> Text@028D9E00 refcount=3<\n> > > > > p@028D3628 refcount=3< Text@028D34F0 refcount=3< \n> > > > I'll add the frame model as an attachment.
Attached file frame model dump (deleted) —
an odd thing about this bug: on rick's machine, it crashes when viewer or mozilla are run normally, but does *not* crash when we step through it in the debugger. rick captured the frame model after running through the debugger with break points in the parser. I don't think this is the interesting part of the problem. In the crash case, it crashes during an incremental reflow (the image load, probably). The debugger might change the timing of when the image is loaded. I think we need to capture the frame model earlier, before the incremental reflow comes through.
the attachment I just added shows the screwy frame model just before we do the reflow that causes the crash. karnaze: you and I should look at this together asap. harish: you and rick should talk about the content model that gets created as a result of the test case.
I think the *primary* bug here, the one that's causing this crash, is the incorrect content model. The table frame construction code is handling the illegal content model poorly, but that is a separate problem. I'm going to assign this bug to harish so he can get the content model straightened out. I'll open a new bug on Chris, because even after Harish's fix an author could still get into trouble manipulating the document via the DOM. Harish, it looks like it's the existance of the unknown tags <sh> that cause your illegal-content-in-table code to do the wrong thing.
Assignee: buster → harishd
Status: ASSIGNED → NEW
submitted bug 57010 for the frame portion of this bug.
Blocks: 57010
At some point there was a consensus that userdefined tags could exist any where in the document and therefore it wasn't necessary for me to treat them as illegal-content inside table. But, since the table code couldn't deal with userdefined tags we can definitely make an exception, in the DTD, and throw them out of the table. Now, the question is, should we deal this problem for rtm?
Tables *should* be able to handle this, but dont -- and we crash. That's really bad, and doable in both html source and via the DOM. I think this is a mandatory fix.
I agree that it's a crasher and should be fixed. But, we should know how many sites are actully affected by this. Without an estimate it will be pretty hard to pass thro' PDT.
Status: NEW → ASSIGNED
Whiteboard: [rtm need info] → [rtm need info][Fix in hand]
Harish, I'm going to work on a patch in table frame construction which may be safer than your patch. Please don't mark this rtm+ until tomorrow
Attached patch frame construction patch (deleted) — Splinter Review
The frame construction patch is a bit larger than I had anticipated. It still may be safer than the parser patch (we need to decide) and it is certainly more general (it will probably handle additional javascript cases). Neither the test case nor the url crashes, but the url is a very large page, and I am getting a ton of assertions in block code when scrolling, so I can't tell if it is rendering correctly.
yes, it is a huge page. But I was not getting any assertions while loading it, while I was trying to minimize the test case. In fact, you can verify this by simply removing the image at the end of the document, thus removing the incremental reflow and preventing the crash.
With the frame construction patch more frames are getting constructed, so it is not too surprising that I am seeing new assertions. If we decide to go with this patch, then we need to get together and understand what is causing the assertions.
Unless we can have a less risky, smaller fix for this bug AND prove that this bug occurs on high visibility sites, there is no way the PDT will allow this fix into the rtm branch. Based on current information, I don't think this bug is high impact enough to warrant fixing it on the rtm branch. Marking rtm-. Please re-nominate if you disagree.
Whiteboard: [rtm need info][Fix in hand] → [rtm-][Fix in hand]
Late in the game, but attaching MacsBug stdlog from crash on this bug. Mac OS 9.0.4, Mac trunk Mozilla installer build 2000102312.
Reassigning to myself.
Assignee: harishd → karnaze
Status: ASSIGNED → NEW
The frame construction patch has been checked in.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified in the 2001010209 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: