Closed Bug 536048 Opened 11 years ago Closed 10 years ago
Cannot distinguish the log for "Empty Trash" from folder deletion in Activity manager
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; ja; rv:220.127.116.11) 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".
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → 3.0
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
The solution is simple. Please add a string for "Empty Trash" log.
Actually, getting "Deleted folder Trash" in the activity manager after emptying the Trash is rather irritating.
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
(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.
Marcel, would you care to attach your patch for this?
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.
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". ;)
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+
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.
You need to log in before you can comment on or make changes to this bug.