314517 - Evil MathML testcase involving mtable {position:absolute} crashes [@ nsLineBox::DeleteLineList] when closing the window/tab

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase.
When closing the tab/window of the testcase, current Mozilla trunk build (2005-10-28) crashes.

Talkback ID: TB11286502X

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Forgot to say, it doesn't crash Mozilla1.7, that's why I added the regression keyword.

Comment 2

[Martijn Wargers (dead)]

Ok, this regressed between 2004-10-26 and 2004-10-27:
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+06%3A00%3A00&maxdate=2004-10-27+08%3A00%3A00&cvsroot=%2Fcvsroot
So my guess would be that this is a regression from fixing bug 263406.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Also in branch: TB11287726G

Comment 4

[Martijn Wargers (dead)]

From talkback ID: TB11286502X

nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
CanvasFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLFrame.cpp, line 229]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsHTMLScrollFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsGfxScrollFrame.cpp, line 174]
ViewportFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsViewportFrame.cpp, line 67]
DocumentViewerImpl::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 1448]
nsSHEntry::~nsSHEntry  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 123]
nsSHEntry::Release  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 130]
DocumentViewerImpl::Show  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 1728]
nsPresContext::EnsureVisible  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresContext.cpp, line 1294]
PresShell::UnsuppressAndInvalidate  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 4697]
PresShell::UnsuppressPainting  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 4745]
nsDocShell::EndPageLoad  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 4783]
nsWebShell::EndPageLoad  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/base/nsWebShell.cpp, line 664]
nsDocShell::OnStateChange  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 4709]
nsDocLoader::FireOnStateChange  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/uriloader/base/nsDocLoader.cpp, line 1210]
nsDocLoader::doStopDocumentLoad  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/uriloader/base/nsDocLoader.cpp, line 844]
nsDocLoader::OnStopRequest  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/uriloader/base/nsDocLoader.cpp, line 665]
nsLoadGroup::RemoveRequest  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsLoadGroup.cpp, line 686]

Comment 5

[Boris Zbarsky [:bzbarsky]]

See http://lxr.mozilla.org/seamonkey/source/layout/base/nsCSSFrameConstructor.cpp#7125 and http://lxr.mozilla.org/seamonkey/source/layout/base/nsCSSFrameConstructor.cpp#7172

Basically, whenever mtable is positioned or floated, all sorts of layout invariants get broken...  If desired, I can probably construct cases where this crashed with 1.7 as well.  ;)

I can perhaps plug up this crash by figuring out exactly what assertions are failing in this case, but the real fix is to fix mtable to work right (or to add styles in mathml.css forcing position:static and float:none on it).

Comment 6

[rbs]

>Basically, whenever mtable is positioned or floated, all sorts of layout
>invariants get broken...

No just mtable, e.g., "Bug 307826 - Crash when floated <html:div> is removed from <math:mi> parent". Just the title of that bug is a contradiction... <mi> is meant to contain a token (i.e., plain text...), not a <div>, let a lone a floated one.

So these issues are really irrelevant to MathML which takes care of other things and its own invariants. All we should care about with this positioning is to not crash with minimum code, and avoid littering unnecessary #ifdef MATHML support to the code because it won't make sense, like attempting to render the square-root of a child, yet that child is position:fixed at the bottom of the screen. Trying to support these becomes an entangled mess and more importantly, they have no meaning and no other renderer will ever support them.

I have some embryonic code aimed at ignoring positioning and floating, which still needs some tidying.

Comment 7

[rbs]

>I can perhaps plug up this crash by figuring out exactly what assertions are
>failing in this case, but the real fix is to fix mtable to work right (or to
>add styles in mathml.css forcing position:static and float:none on it).

It turned out that this is the way to go for every element under the MathML namespace and everything else within a MathML subtree. I don't forsee any use of floating/positioning, even as a service from the C++ back-end, for MathML. Hence it is pretty realistic to disable floating/positioning once for all and avoid the crashes. Plus, the code is kept minimal, with chances to to get into Minimo-based PDAs one day.

I have attached a patch on bug 307826.

Comment 8

[rbs]

https://bugzilla.mozilla.org/show_bug.cgi?id=314517