Cannot distinguish the log for "Empty Trash" from folder deletion in Activity manager

RESOLVED FIXED in Thunderbird 3.1b2

Status

enhancement
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: marsf, Assigned: marcel)

Tracking

({polish})

Thunderbird 3.1b2
Bug Flags:
blocking-thunderbird3.1 -

Firefox Tracking Flags

(blocking-thunderbird3.1 -, thunderbird3.1 beta2-fixed)

Details

(Whiteboard: ucosp)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; ja; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091219 Shredder/3.1a1pre

When I've deleted the normal folder named "trash", Activity manager says: "Deleted folder trash". When I've empty the Trash folder, Activity manager says: "Deleted folder Trash".

They are using same entity "deletedFolder" in activity.properties. I think the log for "Empty Trash" is not proper. Because the Trash folder has not actually deleted.


Reproducible: Always

Steps to Reproduce:
1. Empty the Trash folder.
2. Delete a normal folder named "trash".
3. See the log in Activity manager.
Actual Results:  
Shows same log in Activity manager.

Expected Results:  
When I empty the Trash folder, the log should be separated with folder deletion. It should be "the Trash folder has emptied".
(Reporter)

Updated

10 years ago
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → 3.0
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3.1?
Keywords: polish
(Reporter)

Comment 1

10 years ago
The solution is simple.
Please add a string for "Empty Trash" log.

Comment 2

9 years ago
Actually, getting "Deleted folder Trash" in the activity manager after emptying the Trash is rather irritating.
(Assignee)

Comment 3

9 years ago
I've got a fix for this and am working on learning how tests work before submitting it.

Currently my fix adds a string for "Empty Trash" but it's easy enough to suppress this. What do you guys think?
Assignee: nobody → marcel
Status: NEW → ASSIGNED
Whiteboard: ucosp
Duplicate of this bug: 541090
Duplicate of this bug: 543309
(In reply to comment #3)
> I've got a fix for this and am working on learning how tests work before
> submitting it.
> 
> Currently my fix adds a string for "Empty Trash" but it's easy enough to
> suppress this. What do you guys think?

I'ld add the "Empty Trash" string, but Bryan Clark is our UX guy, so let's see what he says.  :)

Later,
Blake.

Comment 7

9 years ago
Marcel, would you care to attach your patch for this?
blocking-thunderbird3.1: --- → -
Flags: blocking-thunderbird3.1? → blocking-thunderbird3.1-
(Assignee)

Comment 8

9 years ago
This also contains fixes to some related issues that I found while fixing this bug (and couldn't find on Bugzilla). I can split these out into separate patches/bugs if you'd prefer.

1. When renaming a folder, both a delete and a rename event are fired, resulting in two entries in the Activity Manager. My patch removes the delete event.
2. When moving a folder Foo to the trash, the message in the Activity Manager is "Renamed folder foo to foo". I changed it to: "Moved folder foo to trash"

One possible side effect of this change is that renaming a folder within the trash folder (ie. from Trash/foo to Trash/bar) still adds the "Moved folder foo to trash" string. I can't figure out a way to fix this - all information about the original folder's parent is nulled out before calling NotifyFolderRenamed().
(In reply to comment #8)
> Created an attachment (id=427994) [details]
> Adds an Empty Trash string to the Activity Manager

Thanks a lot for the patch. To be able to get your patch into the tree you need to follow a few requirements described at https://developer.mozilla.org/en/comm-central#comm-central_tree_rules. For now you would need to request a review. Your patch lacks a unit test, these are required since January 4th (see http://groups.google.com/group/mozilla.dev.apps.thunderbird/browse_thread/thread/d0690bb47c771568/716d8aa540749c9f?lnk=gst&q=test+policy#716d8aa540749c9f). Will be glad to provide guidance for the test part.
(Reporter)

Comment 10

9 years ago
Thank you Marcel.

FYI, string freeze date is 2010-03-23 (next Tuesday).
When the patch will be reviewed?
Well, it would be a lot more likely to get reviewed if Marcel set the review flag to "?", and the reviewer to ":bwinton".  ;)
Attachment #427994 - Flags: review?(bwinton)
Comment on attachment 427994 [details] [diff] [review]
Adds an Empty Trash string to the Activity Manager

Looks good to me.

Now you need to ask someone else for an additional review.  I suggest either Standard8 or Bienvenu, since they seem to have reviewed these files before.

Later,
Blake.
Attachment #427994 - Flags: review?(bwinton) → review+
Comment on attachment 427994 [details] [diff] [review]
Adds an Empty Trash string to the Activity Manager

Drive by ui-review!

"Emptied Trash" - looks good
"Moved folder #1 to Trash" - capitalize Trash since it's a "special folder"

ui-r+ with that change.
Attachment #427994 - Flags: ui-review+
(Assignee)

Comment 14

9 years ago
Comment on attachment 427994 [details] [diff] [review]
Adds an Empty Trash string to the Activity Manager

Thanks for the reviews guys!
Attachment #427994 - Flags: superreview?(bugzilla)
Attachment #427994 - Flags: superreview?(bugzilla) → superreview+
I've checked this in with Bryan's comment addressed:

http://hg.mozilla.org/comm-central/rev/ac77cab73bd1

Thanks for the patch.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
You need to log in before you can comment on or make changes to this bug.