Steps to reproduce: 1. Open Edit > Find > Search Messages 2. Perform a search so that you have at least one result 3. Try clicking a column header Then it crashes. I could see this on Windows and Linux The regression happened between 2006-09-02 and 2006-09-05. Maybe that's coming from bug 348504. TB22932859W nsMsgDBView::GetCurColumnHandlerFromDBInfo [mozilla/mailnews/base/src/nsMsgDBView.cpp, line 1599] nsMsgDBView::Sort [mozilla/mailnews/base/src/nsMsgDBView.cpp, line 3684] nsMsgSearchDBView::Sort [mozilla/mailnews/base/src/nsMsgSearchDBView.cpp, line 582] XPTC_InvokeByIndex [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102] XPCWrappedNative::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2169] XPC_WN_CallMethod [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1449] js_Invoke [mozilla/js/src/jsinterp.c, line 1353] js_Interpret [mozilla/js/src/jsinterp.c, line 4052] js_Invoke [mozilla/js/src/jsinterp.c, line 1372] nsXPCWrappedJSClass::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1415] nsXPCWrappedJS::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp, line 468] SharedStub [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp, line 147] nsEventListenerManager::HandleEventSubType [mozilla/content/events/src/nsEventListenerManager.cpp, line 1655] nsEventListenerManager::HandleEvent [mozilla/content/events/src/nsEventListenerManager.cpp, line 1762] nsXULElement::HandleDOMEvent [mozilla/content/xul/content/src/nsXULElement.cpp, line 2229] nsXULElement::HandleDOMEvent [mozilla/content/xul/content/src/nsXULElement.cpp, line 2208] nsXULElement::HandleDOMEvent [mozilla/content/xul/content/src/nsXULElement.cpp, line 2208] PresShell::HandleEventInternal [mozilla/layout/base/nsPresShell.cpp, line 6423] PresShell::HandleEventWithTarget [mozilla/layout/base/nsPresShell.cpp, line 6321]

This is most definitely a regression from bug 348504.

This should fix it. My mistake - never realised that the search form uses the same interface which means that m_db would be null.

Comment on attachment 236903 [details] [diff] [review] Check m_db and dbInfo Oren, Would it make sense to do a if (!(m_db || dbInfo)) return nsnull; right at the top instead of having 2 clauses? David's on vacation, I can review this for you.

no - because m_db is used to instansiate dbInfo. If m_db does not exist then I can't pull dbInfo. Basically: First check that m_db exists Then try to get dbInfo Make sure that that exists (second check) etc... Thanks!

I think you should at least move the m_db check to above the dbInfo declartion. (It really seems like it would be good to XPCOM-ify this method; e.g. make it an NS_IMETHODIMP and return rv, etc.)

- reordered the null checks (as per Adam's comment - #5) You are right about the ordering - done. There is an XPCOM method that calls this one. What is the benifit of this one being XPCOMified? It is only ever called internally.

Comment on attachment 236917 [details] [diff] [review] Reordered null checks this will do for now - thx for the patch; I'll land it on the trunk and branch.

yeah, we don't want to xpcomify this function - it's an internal method. More error checking is a good idea, however, and we shouldn't be calling this method at all unless we're doing a byCustom sort...

fixed on trunk and branch

verified on the 1.8 branch using the Thunderbird 2 candidate build, version 2.0.0.0 (20070326). I verified using the reporter's STR. Adding verified keyword for the branch.