476698 - Activity Manager: report on 'send later' progress

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[David Ascher (:davida)]

The activity manager should probably display the progress of sending, as well as indicate the presence of unsent messages.

Comment 1

[Mark Banner (:standard8)]

Attachment: Prototype

This is something I was able to knock up today. It is hooked into the send later progress - at the moment it just shows how many messages are being sent, not progress on the individual messages.

Notes:
- I'm expecting future versions to show progress of the actual message sending, but the send later service doesn't support that at the moment.
- It appears that process activities have to have contextType/Text ? or assume that they do even if you don't set it.
- contextType/Text doesn't seem to apply to events
- I've done some work on hooking up plural strings - they need some more work yet, probably once we figure out the full ui.
- This patch addresses the issue of being able to run one instance of a listener (which passes info to activity manager) across the whole application without needing an xpcom service implementation.

Question:
- If we show progress of sending individual messages and total progress, would we have one contextType (e.g. "Sending Messages") with the individual message sending below them?

I'll attach the current patch in a moment.

Comment 2

[Mark Banner (:standard8)]

Attachment: WIP v1

Comment 3

[Mark Banner (:standard8)]

Note, the patch needs the patch on bug 477054 to work properly.

Comment 4

[Mark Banner (:standard8)]

Attachment: Prototype v2

Updated prototype with revised text and basic display.

This shows the actual send progress of messages and the fact they have completed.

I'm intending on this being a two-stage process, the first one will just get the basic strings and layout working, then the second will add feedback as to who you sent the message to.

Comment 5

[Mark Banner (:standard8)]

Attachment: WIP v2

The patch. I still need to handle error states and check through multiple sends (at the same time), but otherwise it should be working.

Comment 6

[Mark Banner (:standard8)]

Attachment: Initial Version

Here's the first version I'm proposing to get in before the string freeze. Examples of success, failure and in progress are displayed. Its not perfect - there are some progress updates that need to be shown still (e.g. for copying messages to sent folder), but its a good start for showing what we can do.

The strings shown are what I'm intending will be the fallback. Once I can do a bit more work on the interfaces I should be able to get the current message to the listener, and hence the strings will be "Sent Message to Mark Banner" etc.

Note, this will all only be displayed for send unsent messages at the moment, as that is where we are heading in bug 440794.

Comment 7

[Mark Banner (:standard8)]

Attachment: [checked in] The fix

I'd like to get this in before the string freeze (tonight) if possible, and we'll build on it before and after b2. Asking for sr because of the minor mailnews/ change, pinging several people for review as I'm not sure if bienvenu will want to review this, and I don't know who else is about.

Here's the details:

- If you want to test, you'll need the patches from bug 477680 and bug 476700.
- The send progress/event/warning is enabled only for send later activity.
- This patch lays the foundation for other activity manager patches; we set up a send later listener which then feeds the activity manager UI. This keeps the two areas of code distinct. By loading in via a javascript module, we keep one instance regardless of how many windows we open/load the modules into. This stops duplicate entries/management, we just have to ensure that we load the modules for each window we can startup with.
- There are a couple of fixes to activityManager.js - I realised we'd got a couple of things wrong in the initial patch.
- Not all the strings are in use yet (e.g. the *To), I plan to use them in the next phase which should be before b2 and will give an indication of who the email was sent to.

Comment 8

[David :Bienvenu]

I'm looking through this and will try to apply/run it soon.

+    let displayText;
+
+    if (aTo) {
+      displayText = this.bundle.formatStringFromName("sendingMessageTo",
+                                                     [aTo], 1);
+    }
+    else {
+      displayText = this.bundle.GetStringFromName("sendingMessage");
+    }

this can be one statement:

let displayText = aTo ? ...

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 361510 [details] [diff] [review]
[checked in] The fix

David says he's happy reviewing this.

Comment 10

[David :Bienvenu]

Comment on attachment 361510 [details] [diff] [review]
[checked in] The fix

r/sr=me, modulo the nit I mentioned before, replaceProcessWithEvent -> _replaceProcessWithEvent, and we'll deal with the fact that clearing the AM list doesn't survive closing and re-opening the AM.

Comment 11

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

Comment on attachment 361507 [details]
Initial Version

Looks good.  I'm sure we'll have some cleanups later; once we get some testing.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 361510 [details] [diff] [review]
[checked in] The fix

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

I filed bug 477862 about the clear list button issue.

I'll be leaving this bug open for the hooking up of the To %S part of the strings.

See bug 257942 comment 70 for how to access the activity manager on nightlies if you want to see this in action (note: we only have Mac themes at the moment).

Comment 13

[Mark Banner (:standard8)]

In bug 440794 we actually went with the subject, rather than who the message was to. Some of the UI was redone as well.

I think this is now fixed - future work on send in background/send later will reshape as necessary, but we don't need to keep this bug open for that.