Using May14 builds on Linux Redhat 5.2, Mac OS8.51 and NT 4.0 Crash after deleting last message in thread pane then Shift Selecting in thread pane. 1. Using prefs with a single POP account. Open a folder having a few messages (due to bug #6426 there is no scroll bar to scroll to end of longer thread pane listing). 2. Select the message appearing at the bottom/end of thread pane list. Delete it. 3. Shift+Select and click another message in the thread pane. Result: crash occurs.

Laurel - did you happen to get a talkback report?

No, I'm not seeing Talback engage. I'm working on why it isn't engaging.

Reassign to putterman as P1 for M6.

reassigning to hyatt. This is the call stack: nsTreeCellFrame::Select(nsIPresContext & {...}, int 1, int 1) line 271 + 7 bytes nsTreeFrame::RangedSelection(nsIPresContext & {...}, nsTreeCellFrame * 0x042b4380) line 147 nsTreeCellFrame::HandleMouseDownEvent(nsIPresContext & {...}, nsGUIEvent * 0x0012fca8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 188 nsTreeCellFrame::HandleEvent(nsTreeCellFrame * const 0x042b4380, nsIPresContext & {...}, nsGUIEvent * 0x0012fca8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 163 PresShell::HandleEvent(PresShell * const 0x03d9cea4, nsIView * 0x03e8e1e0, nsGUIEvent * 0x0012fca8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 1993 + 38 bytes nsView::HandleEvent(nsView * const 0x03e8e1e0, nsGUIEvent * 0x0012fca8, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 833 nsView::HandleEvent(nsView * const 0x03e8eac0, nsGUIEvent * 0x0012fca8, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 820 nsView::HandleEvent(nsView * const 0x03e8eb50, nsGUIEvent * 0x0012fca8, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 820 nsView::HandleEvent(nsView * const 0x03d9b550, nsGUIEvent * 0x0012fca8, unsigned int 28, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 820 nsViewManager::DispatchEvent(nsViewManager * const 0x03d9b770, nsGUIEvent * 0x0012fca8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 1726 HandleEvent(nsGUIEvent * 0x0012fca8) line 67 nsWindow::DispatchEvent(nsWindow * const 0x03e8e9b4, nsGUIEvent * 0x0012fca8, nsEventStatus & nsEventStatus_eIgnore) line 410 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fca8) line 431 nsWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000 {x=??? y=???}) line 2881 + 15 bytes ChildWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000 {x=??? y=???}) line 3030 nsWindow::ProcessMessage(unsigned int 513, unsigned int 5, long 13762722, long * 0x0012fe48) line 2238 + 24 bytes nsWindow::WindowProc(HWND__ * 0x00020e66, unsigned int 513, unsigned int 5, long 13762722) line 474 + 27 bytes Without knowing what's going on, I'm guessing that when the selected item is deleted, the selection isn't getting cleared. It works everywhere else because there's an item taking the deleted item's place. But when the last one is deleted, there's no item that takes its place.

I'm not handling deletion's impact on the front-end frame selection yet. Oops. This could take a day or so to fix. I'll try to knock it out for M6.

Fixed.

REOPEN -- Still crashing using 1999051809 on Linux Redhat 5.2 OK using 1999051808 on NT 4.0 Can't use today's mac build for verification (won't launch).

I take that back -- NT 4.0 does crash, too. A couple times it didn't using today's build, but now it's doing so. Might this mean the fix isn't in the 18th's builds? If so, I'll try in tomorrow's.

David, I'm getting the same crash with the same stack trace in my debug build that was updated after you checked in your changes.

I'm cc'ing buster and karnaze on this bug. It appears that, when a table row is removed, the table cell frame never gets deleted. I stepped through the code a bit, and I don't see where (if ever) the table code is calling DeleteFrames or even DeleteFrame on the removed frames. This seems wrong. Specifically IR_RowRemoved and IR_CellRemoved just remove the frames from the flow. I don't see any calls to actually delete the frames. Would it be wrong of me to just throw some delete calls into those functions? This is an M6 bug, so rapid response is appreciated. Thanks guys.

I believe this used to work, then sometime early this year the code was switched over to use nsFrameList. Since nsFrameList was added after I stopped working on table frames, I'm not familiar with its semantics. Looking at it, though, it sure seems like incremental reflow methods that handle deleted content should all be calling DeleteFrame rather than RemoveFrame. I'd certainly try that for all the IR_*Removed methods in the various table frame classes.

do we need to hold m6 for this?

I'm moving to M7. I need a comment from the current table owner (karnaze). I don't feel comfortable messing around with the table code when I'm not sure what the right thing to do is.

I'm reassigning this to karnaze and marking as dogfood. Chris, the problem seems to be that the table code doesn't actually destroy any frames when you start removing rows and cells from the table. I need to have my DeleteFrame methods in the tree widget called, so that I can update and track my selection. Otherwise the tree gets out of sync and crashes. Right now if you remove a row, no DeleteFrame method is called. I need to have a DeleteFrame method called recursively on the row and on the cells. If DeleteFrame is not the right method to use, let me know which one is. I believe it is the right method, though, and that the table code is leaking frames when rows or cells are removed.

*** Bug 7803 has been marked as a duplicate of this bug. ***

Adding myself and Paul to CC because we both need this for our respective UI.

I need more detail on how to reproduce this bug. I do 95% of my testing in Viewer. I don't understand the language of this bug starting with item 1. I don't know how to hook up mail news or run it. Please spoon feed me if you want it fixed. Or better yet, give me a Viewer test that has the same problem.

See http://www.mozilla.org/mailnews/prefs-info.html for how you can set up mail. If you need help, let me know. I don't have a non-mail news tests cas, but maybe someone else has one.

I was not able to bring up a functional mail/news window from apprunner. However, using layout/tests/TableIncrementalReflow.html I verified that the frames were indeed getting removed but not destroyed. I changed them to call DeleteFrame instead of RemoveFrame and now the row frames and the cell frames (and all other table related frames) get deleted. Someone, please verify that the original mail/news problem is fixed.

Unfortunately it looks like it got worse. I only rebuilt layout, so if there are other dependencies, that could be the problem. But now I crash when I hit delete (before I even try doing shift-select) with the stack below 850cc483() nsJSUtils::nsConvertObjectToJSVal(nsISupports * 0x00129e48, JSContext * 0x02fe34f0, long * 0x0012a564) line 133 + 20 bytes GetEventProperty(JSContext * 0x02fe34f0, JSObject * 0x035b4528, long -3, long * 0x0012a564) line 101 + 17 bytes js_GetProperty(JSContext * 0x02fe34f0, JSObject * 0x035b4528, long 51009360, long * 0x0012a564) line 1694 + 72 bytes js_Interpret(JSContext * 0x02fe34f0, long * 0x0012a6e4) line 2155 + 978 bytes js_Invoke(JSContext * 0x02fe34f0, unsigned int 1, int 0) line 666 + 13 bytes js_CallFunctionValue(JSContext * 0x02fe34f0, JSObject * 0x00e30130, long 14876992, unsigned int 1, long * 0x0012a828, long * 0x0012a830) line 735 + 15 bytes JS_CallFunctionValue(JSContext * 0x02fe34f0, JSObject * 0x00e30130, long 14876992, unsigned int 1, long * 0x0012a828, long * 0x0012a830) line 2554 + 29 bytes nsJSEventListener::HandleEvent(nsIDOMEvent * 0x037b74c0) line 97 + 34 bytes nsEventListenerManager::HandleEvent(nsIPresContext & {...}, nsEvent * 0x0012a9e0, nsIDOMEvent * * 0x0012a9b4, unsigned int 3, nsEventStatus & nsEventStatus_eIgnore) line 882 + 21 bytes RDFElementImpl::HandleDOMEvent(RDFElementImpl * const 0x0311ba90, nsIPresContext & {...}, nsEvent * 0x0012a9e0, nsIDOMEvent * * 0x0012a9b4, unsigned int 1, nsEventStatus & nsEventStatus_eIgnore) line 2279 nsTreeFrame::FireChangeHandler(nsIPresContext & {...}) line 266 nsTreeFrame::RemoveFromSelection(nsIPresContext & {...}, nsTreeCellFrame * 0x03887a00) line 181 nsTreeCellFrame::DeleteFrame(nsTreeCellFrame * const 0x03887a00, nsIPresContext & {...}) line 342 nsFrameList::DeleteFrames(nsIPresContext & {...}) line 29 nsContainerFrame::DeleteFrame(nsContainerFrame * const 0x03887b20, nsIPresContext & {...}) line 82 nsFrameList::DeleteFrame(nsIPresContext & {...}, nsIFrame * 0x03887b20) line 115 nsTableRowGroupFrame::IR_RowRemoved(nsTableRowGroupFrame * const 0x037f4160, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, RowGroupReflowState & {...}, unsigned int & 0, nsTableRowFrame * 0x03887b20) line 1329 + 19 bytes nsTableRowGroupFrame::IR_TargetIsMe(nsTableRowGroupFrame * const 0x037f4160, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, RowGroupReflowState & {...}, unsigned int & 0) line 1156 + 35 bytes nsTableRowGroupFrame::IncrementalReflow(nsTableRowGroupFrame * const 0x037f4160, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, RowGroupReflowState & {...}, unsigned int & 0) line 1086 + 31 bytes nsTableRowGroupFrame::Reflow(nsTableRowGroupFrame * const 0x037f4164, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 995 + 35 bytes nsContainerFrame::ReflowChild(nsIFrame * 0x037f4160, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 392 + 28 bytes nsTableFrame::IR_TargetIsChild(nsTableFrame * const 0x03121560, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, InnerTableReflowState & {...}, unsigned int & 0, nsIFrame * 0x037f4160) line 3582 + 34 bytes nsTableFrame::IncrementalReflow(nsTableFrame * const 0x03121560, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 3176 + 35 bytes nsTableFrame::Reflow(nsTableFrame * const 0x03121564, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 2502 + 35 bytes nsContainerFrame::ReflowChild(nsIFrame * 0x03121560, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 392 + 28 bytes nsTableOuterFrame::IR_InnerTableReflow(nsTableOuterFrame * const 0x03121670, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState & {...}, unsigned int & 0) line 628 + 34 bytes nsTableOuterFrame::IR_TargetIsInnerTableFrame(nsTableOuterFrame * const 0x03121670, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState & {...}, unsigned int & 0) line 420 + 31 bytes nsTableOuterFrame::IR_TargetIsChild(nsTableOuterFrame * const 0x03121670, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState & {...}, unsigned int & 0, nsIFrame * 0x03121560) line 392 + 31 bytes nsTableOuterFrame::IncrementalReflow(nsTableOuterFrame * const 0x03121670, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, OuterTableReflowState & {...}, unsigned int & 0) line 377 + 35 bytes nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x03121674, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1027 + 35 bytes nsBlockReflowContext::ReflowBlock(nsIFrame * 0x03121670, const nsRect & {x=0 y=0 width=10035 height=1073741824}, int 1, int 0, int 1, nsMargin & {top=0 right=0 bottom=0 left=0}, unsigned int & 0) line 227 + 42 bytes nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 0x03130d10, int * 0x0012b6d4) line 2493 + 56 bytes nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x03130d10, int * 0x0012b6d4) line 1983 + 20 bytes nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 1793 + 20 bytes nsBlockFrame::Reflow(nsBlockFrame * const 0x0311a4a4, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1199 + 18 bytes nsAreaFrame::Reflow(nsAreaFrame * const 0x0311a4a4, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 265 + 25 bytes nsContainerFrame::ReflowChild(nsIFrame * 0x0311a4a0, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 392 + 28 bytes RootFrame::Reflow(RootFrame * const 0x0311a664, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 229 nsContainerFrame::ReflowChild(nsIFrame * 0x0311a660, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 392 + 28 bytes nsScrollFrame::Reflow(nsScrollFrame * const 0x03119174, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 423 nsContainerFrame::ReflowChild(nsIFrame * 0x03119170, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 392 + 28 bytes ViewportFrame::Reflow(ViewportFrame * const 0x03119634, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 438 nsHTMLReflowCommand::Dispatch(nsHTMLReflowCommand * const 0x037b7720, nsIPresContext & {...}, nsHTMLReflowMetrics & {...}, const nsSize & {width=10275 height=4920}, nsIRenderingContext & {...}) line 169 PresShell::ProcessReflowCommands(PresShell * const 0x030556a0) line 1236 PresShell::ExitReflowLock(PresShell * const 0x030556a0) line 655 PresShell::ContentRemoved(PresShell * const 0x030556a8, nsIDocument * 0x0301e170, nsIContent * 0x037f49c0, nsIContent * 0x03855080, int 49) line 1688 XULDocumentImpl::ContentRemoved(XULDocumentImpl * const 0x0301e170, nsIContent * 0x037f49c0, nsIContent * 0x03855080, int 49) line 1948 RDFElementImpl::RemoveChildAt(RDFElementImpl * const 0x037f49c0, int 49, int 1) line 1648 RDFTreeBuilderImpl::RemoveWidgetItem(nsIContent * 0x037f49c0, nsIRDFResource * 0x0301e590, nsIRDFResource * 0x0381c4f0) line 1331 + 18 bytes RDFGenericBuilderImpl::OnUnassert(RDFGenericBuilderImpl * const 0x0311e734, nsIRDFResource * 0x031e0370, nsIRDFResource * 0x0301e590, nsIRDFNode * 0x0381c4f0) line 1396 + 37 bytes CompositeDataSourceImpl::OnUnassert(CompositeDataSourceImpl * const 0x0311b5c4, nsIRDFResource * 0x031e0370, nsIRDFResource * 0x0301e590, nsIRDFNode * 0x0381c4f0) line 1201 nsMsgRDFDataSource::unassertEnumFunc(void * 0x0311b5c4, void * 0x0012e020) line 311 nsVoidArray::EnumerateForwards(int (void *, void *)* 0x012032a0 nsMsgRDFDataSource::unassertEnumFunc(void *, void *), void * 0x0012e020) line 213 + 20 bytes

No, this is good. This means the problem is mine now rather than karnaze's. It looks like my tree cell deletion code is getting called now. Reopening and assigning to me.

I'm not seeing this crash in mail. Could you give me the steps that reproduced this?

I crash as soon as I delete a message. My tree is updated as of this morning so it's possible something else could have changed since then. 1. Open the 3 pane 2. Select a folder 3. Select a message in that folder 4. Hit the delete button and crash.

I believe karnaze checked in a fix tonight.

I should have said it's a tree from this morning plus karnaze's fix. If you're not seeing it, I'll do a full update tomorrow morning and verify this.

After pulling the tree and rebuilding I'm still crashing. Admittedly, I haven't done a clobber build, but deleting a message still causes the crash with the stack I put above.

The delete problem Scott Putterman mentions occurs in today's release build of Win32 only (other platforms are OK). I'm going to track that delete crash under a separate bug and mark this bug dependent on it if folks cannot look into this bug until the delete problem is fixed. Thanks. This way, this bug 6459 will be used to track the original reported problem.

I've decided to back out my use of "onchange" in threadPane.xul and use "onclick" instead like I did before. When these bugs are fixed I'll go back to "onchange". If you need an example, go to mailnews/base/resources/content/threadPane.xul and change the tree's "onclick" to "onchange". Unfortunately, I'm still crashing on exit when I go back to "onclick". However, the original buck that was assigned to karnaze about not being able to shift-select after deleting the last message appears to be fixed.

One thing to realize, Scott, is that you can't just switch the "onclick" to being an "onchange". They don't mean the same thing. For example, in your click handler, you're probably trying to display a message, but you don't necessarily want to do that in an onchange handler (which is just tracking selection changes). Also, for onchange, the "target" field of the event object isn't going to be defined (or if I do define it, it will end up being the tree itself and not an individual node). So be careful about making the switch, and don't just replace the word "click" with "change". :) I'm marking this bug fixed, since I believe new ones have been opened regarding the other problems.

Yes, you're probably right about the onchange. I need to check to make sure there is a selection before doing something with it. But, I think I do want to use onchange instead of onclick for handling when to display new messages since it seems to me that selection change allows me to know when to show no message (i.e. when changing folders) and it prevents me from going through the code to display a message when the item clicked on is already selected. At this point though, I don't think the crash was going through my code. But I agree that the original bug was fixed.

Actually, I take back what I last wrote. It might be going through my js code and because the target node is undefined, it may be crashing. The js code probably shouldn't crash, but if I fix it up to not depend on the target node, then all of the problems might be solved.

The original crash was happening because of an undefined target. I'm not setting it for onchange handlers (this is tricky, since that target actually gets set somewhere else, so I guess I'm going to have to do it myself explicitly or something).

The original crash looks to be fixed. OK using jun15 respin (1999061514) on linux, mac and NT.