499265 - Activity manager should coalesce mail move/copy/delete events.

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mark Banner (:standard8)]

+++ This bug was initially created as a clone of Bug #493397 +++

Lots of standard behaviors (deleting a bunch of mail one at a time), gloda syncing autosynced or moves messages) currently result in the activity manager showing huge long lists of essentially duplicate messages.  I think we need to somehow get those things coalesced, so that 100 successive deletion operations are displayed as "100 messages deleted", and so that periodic operations either get summarized ("since we started, 12341 messages indexed") or archived.

The con:
 - coalescing means it becomes impossible to reach in and do something to one of the events in the coalition, such as an eventual undo.  "Luckily", our undo story is weak enough that I think we can not bother for TB3.

Required implementation for this bug:

 - user-triggered actions should be grouped by folder, as long as they're contiguous in the activity manager history.  So that if I do 
    - delete 1 message from inbox
    - delete 1 message from inbox
    - delete 1 message from inbox
    - delete 1 message from inbox
then i switch to todo
    - delete 1 message from ToDo
then back to inbox
    - delete 1 message from inbox
    - delete 1 message from inbox

that should show up as:

    - deleted 4 message from inbox (most recent)
    - delete 1 message from ToDo
    - deleted 2 messages from inbox

The most sensible place to do this it seems would be in http://mxr.mozilla.org/comm-central/source/mail/components/activity/modules/moveCopy.js

Blake, could you take this on?

Comment 1

[Blake Winton (:bwinton) (:☕️)]

Yeah, sure.  Seems like a fun one.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Attachment: Should we coalesce the top two events?

The message is the same, so I kind of think we should, but they occurred in different folders, so maybe it would be better to change the message to reflect that fact.  (i.e. "Deleted 6 messages from Trash"/"Deleted 1 message from Inbox".)

(I've got a patch that coalesces delete events, and it should be fairly quick to update to get either behaviour.)

Thanks,
Blake.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch, assuming that Bryan wants me to change the deleted message. ;)

I don't know if this is the cleanest way to do it.  It seemed like there was a lot of almost-duplication between the Delete and CopyMove cases.  Still, it seems to work.

Thanks,
Blake.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 385830 [details] [diff] [review]
A patch, assuming that Bryan wants me to change the deleted message.  ;)

yes :)

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 385830 [details] [diff] [review]
A patch, assuming that Bryan wants me to change the deleted message.  ;)

>       let statusText = '';
>       // TODO: localize this string

I've just noticed this comment. I think it is out of date now, so we can drop it.

>-# LOCALIZATION NOTE (deletedMessages): #1 number of messages
>-deletedMessages=Deleted #1 message;Deleted #1 messages
>+# LOCALIZATION NOTE (deletedMessages): #1 number of messages, #2 folder name
>+deletedMessages=Deleted #1 message from #2;Deleted #1 messages from #2

As we are changing the context of the string, we have to change the string name as well - this is so that localisers can notice the fact that we've changed the string. Therefore we can just rename deletedMessages to deletedMessages2 here and in the code.

r=Standard8 with that fixed.

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, updated with Standard8's suggestions.

(In reply to comment #5)
> >       // TODO: localize this string
> I've just noticed this comment. I think it is out of date now, so we can drop
> it.

Dropped.

> >-deletedMessages=Deleted #1 message;Deleted #1 messages
> >+deletedMessages=Deleted #1 message from #2;Deleted #1 messages from #2
> As we are changing the context of the string, we have to change the string
> name as well - this is so that localisers can notice the fact that we've
> changed the string. Therefore we can just rename deletedMessages to
> deletedMessages2 here and in the code.

Fixed.

> r=Standard8 with that fixed.

Excellent, thank you!

Later,
Blake.

Comment 7

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/d971def57e3c