Closed Bug 457052 Opened 13 years ago Closed 13 years ago
try to repair cause of assertion - Borked message header
Dan, especially, has been seeing a lot of these. The problem is that a message's thread parent isn't valid, i.e., it's not in the thread. The upcoming patch attempts to repair this situation, and changes the assertion to a warning. The assertion appears harmless, but I think in some situations it could cause us not to display sub-messages in threaded view (not sure, though). This patch repairs the problem for me. I still have to figure out why this is happening, however.
I believe we decided we wanted this for 3.0b1; marking as such.
Target Milestone: --- → Thunderbird 3.0b1
Running with this patch, I'm seeing a small number of the "Thread is a parent of itself" assertions (I was seeing them before, too, I think). Should that really be an assertion, or should it be similar to the problem already solved in this patch and warn and rebuild the index instead...
Comment on attachment 340385 [details] [diff] [review] proposed fix This is making the method big enough that it's becoming difficult to digest. How about splitting it apart?
Attachment #340385 - Flags: superreview?(dmose) → superreview-
ok, this moves the code into a new method. I don't have a test case anymore, though, so I can't swear that this works. Dan, can you try this out and see if you hit the code, and it fixes the problem?
(In reply to comment #1) > I believe we decided we wanted this for 3.0b1; marking as such. Correcting flag to get it onto the right radar.
Flags: blocking-thunderbird3.0b1+ → blocking-thunderbird3+
Comment on attachment 340749 [details] [diff] [review] proposed fix - checked in This looks ok to me.
Attachment #340749 - Flags: review?(bugzilla) → review+
Whiteboard: [needs new patch] → [needs review dmose]
Comment on attachment 340749 [details] [diff] [review] proposed fix - checked in * It would be good to track stuff that should just warn and trigger database rebuilds once rkent's lossless-database-rebuild stuff is in, such as presumably the "Thread is parent of itself" assertion (and maybe the "no root hdr" assertion too?). Please file a spinoff bug, if you would... >+ nsMsgHdr* curMsgHdr = static_cast<nsMsgHdr*>(curHdr.get()); // closed system, cast ok Please reformat so that this wraps at 80 columns. sr=dmose
Attachment #340749 - Flags: superreview?(dmose) → superreview+
fix checked in. I'll file a spin-off bug, but I'm leaning towards repair as opposed to rebuilding.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
This fixes a few other borked thread issues: 1. If a message is marked as the thread root key, but doesn't have a thread parent of -1, make GetRootHdr return the correct header (which will correct the thread root key) 2. If a message is in a thread object, but doesn't have its thread id set to the thread, correct it. 3. If a message is its own parent, reparent it in the thread. 4. In nsMsgThread::ReparentMsgsWithInvalidParent, make sure we don't set our parent to ourselves. I'm not sure the last change is actually needed. The other changes fix the test cases I've got, I believe. I'll double check that.
re-opening, and putting in beta 1.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 3.0a3 → Thunderbird 3.0b1
See bug 272527, which bienvenu thinks is fixed by the patch here. There's an old attachment with an INBOX.msf, too.
Attachment #343621 - Flags: superreview?(neil) → superreview+
Comment on attachment 343621 [details] [diff] [review] fix some other related issues - checked in >+ PRInt32 resultIndex; >+ nsMsgKey rootKey; >+ nsCOMPtr <nsIMsgDBHdr> root; >+ GetRootHdr(&resultIndex, getter_AddRefs(root)); >+ root->GetMessageKey(&rootKey); sr=me if you replace this and just use m_threadRootKey instead.
Attachment #343621 - Flags: review?(bugzilla) → review+
Whiteboard: [needs review dmose] → [needs comment addressing & landing]
fix checked in.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Attachment #340749 - Attachment description: prposed fix → proposed fix - checked in
Attachment #343621 - Attachment description: fix some other related issues → fix some other related issues - checked in
Whiteboard: [needs comment addressing & landing]
You need to log in before you can comment on or make changes to this bug.