This bug was filed from the Socorro interface and is
report bp-8ff7273d-2c34-43a8-97b9-e42fd2130110 .
0 xul.dll nsImapOfflineTxn::UndoTransaction mailnews/imap/src/nsImapUndoTxn.cpp:551
1 xul.dll nsTransactionItem::UndoTransaction editor/txmgr/src/nsTransactionItem.cpp:181
2 xul.dll nsTransactionItem::UndoChildren editor/txmgr/src/nsTransactionItem.cpp:227
3 xul.dll nsTransactionItem::UndoTransaction editor/txmgr/src/nsTransactionItem.cpp:171
4 xul.dll nsTransactionManager::UndoTransaction editor/txmgr/src/nsTransactionManager.cpp:134
5 xul.dll nsMessenger::Undo mailnews/base/src/nsMessenger.cpp:1468
crash first appears in Thunderbird 14
The code where it crashe looks like this:
nsCOMPtr<nsIMsgDBHdr> firstHdr = m_srcHdrs; <---
I wonder if we are sure at least one element exists in the array m_srcHdrs.
Many (all?) functions in the file either append elements, or loop over them as 'for (i=0;i<m_srcHdrs.Count();i++)' so they cope with 0 elements fine. Shouldn't we do that here too? If we really don't want to undo all (the for loop) but only one, then at least check if it exists?
I don't really know the offline imap code, sorry.
I wouldn't object to checking the length of the array, though I don't quite see the point of creating a transaction with nothing to do.
Maybe David Bienvenu could know.
bulletproofing this seems like the right thing to do, perhaps with an assertion, since as Neil says, it shouldn't happen.
Created attachment 709520 [details] [diff] [review]
Comment on attachment 709520 [details] [diff] [review]
Ok, this looks fine to me, we'll let it ride the trains.
m_srcHdrs.Count() < 1 could be m_srcHdrs.IsEmpty().
Created attachment 730305 [details] [diff] [review]
Comment on attachment 730305 [details] [diff] [review]
Taking for next esr as this seems like a stable fix.