Empty of Local Trash doesn't appear to stay empty in the same session

RESOLVED FIXED in Thunderbird 3.0b3

Status

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: standard8, Assigned: Bienvenu)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Thunderbird 3.0b3
regression
Bug Flags:
blocking-thunderbird3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
STR:

1) Have some messages in your local folders trash folder.
2) Select the trash folder.
3) Right click, select Empty Trash.
4) Select a different folder.
5) Select the trash folder again.

Expected results: Empty Trash folder

Actual results: messages are still listed

Selecting a message does nothing.

There are messages listed in the folder until I restart (unless I select one just after step 5, then that is still listed after the restart).

Pretty sure this is a regression from bug 497199.
(Assignee)

Comment 1

9 years ago
taking; I'll look at this. File | Empty trash works, so it's just the context menu one. We are getting to the confirmation prompt...
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
(Assignee)

Comment 2

9 years ago
It feels like someone is holding onto a db, and not letting go when told to...which is a bit surprising to be caused by bug 497199, but we'll see.
(Assignee)

Comment 3

9 years ago
I think this is caused by the trash folder having a null msgDatabase by the time we try to empty it, which means we don't call ForceClosed(), which means anyone holding onto the db will break the removal.  So I could change nsMsgLocalMailFolder::Delete() to re-open the db, if it's null, or I could take this opportunity to add a method to the db service that will tell you if a db is open or not, which I've been meaning to do anyway.
(Assignee)

Comment 4

9 years ago
Created attachment 383330 [details] [diff] [review]
proposed fix

I'll go write a test case for this, but while I'm doing that, if you get a chance, I'd love to know if my diagnosis is right, and that this fixes the problem you're seeing.
Attachment #383330 - Flags: superreview?(bugzilla)
Attachment #383330 - Flags: review?(bugzilla)
(Assignee)

Comment 5

9 years ago
Created attachment 383376 [details] [diff] [review]
test case

this fails w/o the patch, and succeeds with it, for me...
Attachment #383376 - Flags: review?(bugzilla)
(Reporter)

Comment 6

9 years ago
Comment on attachment 383376 [details] [diff] [review]
test case


>+function doTest(test)
>+{
...
>+  else
>+  {
>+    do_test_finished(); // for the one in run_test()
>+  }
>+}

You need to add:

gMsgHdrs = null;

just before the do_test_finished() to avoid reporting morkObject and morkRowObject leaks.
Attachment #383376 - Flags: review?(bugzilla) → review+
(Reporter)

Updated

9 years ago
Attachment #383330 - Flags: superreview?(bugzilla)
Attachment #383330 - Flags: superreview+
Attachment #383330 - Flags: review?(bugzilla)
Attachment #383330 - Flags: review+
(Reporter)

Comment 7

9 years ago
Comment on attachment 383330 [details] [diff] [review]
proposed fix

>+NS_IMETHODIMP nsMsgDBService::CachedDBForFolder(nsIMsgFolder *aFolder, nsIMsgDatabase **aRetDB)
>+{
>+  NS_ENSURE_ARG_POINTER(aFolder);
>+  NS_ENSURE_ARG_POINTER(aRetDB);
>+  *aRetDB = (nsMsgDatabase *) nsMsgDatabase::FindInCache(aFolder);
>+  return NS_OK;
> }

I don't think you need the case here - FindInCache is already returning an nsIMsgDatabase (and you're casting it to the wrong thing anyway).

r/sr=Standard8 with that fixed.
(Assignee)

Comment 8

9 years ago
fix checked in, w/ comments addressed.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Flags: in-testsuite+
(Assignee)

Updated

9 years ago
Flags: blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b3
You need to log in before you can comment on or make changes to this bug.