Closed Bug 489588 Opened 12 years ago Closed 12 years ago

Hook activity manager into the status bar

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: standard8, Assigned: standard8)

References

(Blocks 1 open bug)

Details

(Whiteboard: [b3ux][m6][has patch][needs review clarkbw])

Attachments

(2 files, 2 obsolete files)

Rather than do a full-blown interactive status bar (bug 476487), we're thinking that hooking the activity manager into the existing status bar may be enough to give us the information we need to give some feedback to the user.

I'm going to try and do this patch in two stages - firstly hook up text output, secondly hook up the progress display.
First step, this makes it so that when activity manager gets an event, we show it on the status bar, until the next display of the status bar.

Current this has the following effects:

- When sending a message in the background (or via send later), we'll actually get two notifications on the status bar: "Deleted 1 Message" and "Sent Message". The first of these is due to the fact that we've deleted a message from the outbox. It won't really be seen unless the message is a big one - and I'm planning that when we hook up processes, we'll be getting a "Copying Message..." notification straight away.

- Moved, Copied and Deleted message notifications may get displayed on the status bar. This only occurs if a message hasn't been loaded after the operation (due to the fact that we display "Loading Message..." when selecting a new message).

- You'll get notifications like "Test Account is up to date" from auto sync when new mail is received or auto sync happens.

I think this is a useful first step, especially the sending message as it lets people know that a message has actually been sent.

My plan for the next step is to see if I can hook up nsIActivityProcess indications to the status bar text, such that we give more details of ongoing activities. However that looks slightly more complicated to refine, and I think it would be good to get this patch in for the test day tomorrow.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #374162 - Flags: ui-review?(clarkbw)
Attachment #374162 - Flags: review?(bienvenu)
Attachment #374162 - Flags: review?(bienvenu) → review+
Attachment #374162 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 374162 [details] [diff] [review]
[checked in] Hook activity manager event notifications to status bar

Checked in: http://hg.mozilla.org/comm-central/rev/cd856094e96f
Attachment #374162 - Attachment description: Hook activity manager event notifications to status bar → [checked in] Hook activity manager event notifications to status bar
This patch hooks process activities to the status bar. I'm not intending on doing a patch for warnings in activity manager as we have other methods (dialog, poptarts) for displaying those currently (we can always do them later).

In this patch, I listen for process activities being created/destroyed. When they are created I hook into their progress updates.

Here's the effects on the UI:

- Status bar text will now show either a) the status text from the first active process, or b) the display text from the first active process.

"First" refers to the first process activity that is listed and in progress.

I use the status text of the activity as I think that'll be slightly more descriptive of what is generally happening on the whole.

The status bar will show either the existing text (i.e. existing mechanism), or the activity text depending on which one was received last.

- Progress bar on the status bar will now merge in the activity manager progress as follows ("other processes" refers to processes not currently hooked into activity manager and are using the existing status bar feedback mechanism):

If no process activities and no other processes are running => no progress.

If other processes => show progress (either undetermined or progress %).

If process activities => show progress (either undetermined or progress %).

If process activities and other processes => show progress, use progress % if at least one process has a known %, otherwise show undetermined.

These should all revert to something not showing progress once complete.

Note that when sending in background we "hang" on the progress updates (saying "Sending Message") at the end of the copy stage - this is because the send later service doesn't know about the updates. I've got a patch in progress to fix that.



I think with the previous patch and this patch we should be able to drop bug 476487 (interactive status bar) from the blocking list, as what we have here will be enough to keep us going.
Attachment #374336 - Flags: ui-review?(clarkbw)
Attachment #374336 - Flags: review?(bienvenu)
Correct patch this time...
Attachment #374336 - Attachment is obsolete: true
Attachment #374337 - Flags: ui-review?(clarkbw)
Attachment #374337 - Flags: review?(bienvenu)
Attachment #374336 - Flags: ui-review?(clarkbw)
Attachment #374336 - Flags: review?(bienvenu)
What should search be doing in terms of showing its activity in the status bar?  For example, quick search likes to poke things into the status bar.
Attachment #374337 - Flags: review?(bienvenu) → review+
Comment on attachment 374337 [details] [diff] [review]
Hook process activities to the status bar v2

I've run with this a bit and status and meteors look OK...
(In reply to comment #5)
> What should search be doing in terms of showing its activity in the status bar?
>  For example, quick search likes to poke things into the status bar.

That's a good question, obviously in the short term quick search (and possibly
your new search) should keep doing the same thing.

A quick two-minute think leads me to expect that we're not going to want to
show search as an activity process.

Maybe what we need in the long term as we head towards interactive status bar
is a priority and process based system. So effectively status bar sees
different processes as different processes, rather than everyone just poking
the same bit. These could then be prioritised in terms of what should be
displayed.
Maybe in the long run, search doesn't need to use the status bar at all.  clarkbw?

Right now, quick search uses the meteors to show it is searching, and it puts up a result count (or a no result message) in the status bar.  I have introduced the concept of 'thinking' on a per-tab basis, so the tab could do its own meteor thing, much like firefox does when a tab is loading.  We wouldn't need the global meteor then.  In terms of saying "I found 3 things" or "I found nothing", we could probably use our existing Unread/Total status bar entries instead, or be more direct about it.  The status bar may historically be used for such things in applications, but it's not intuitive unless you're used to it.
I'm getting an error with this patch: failed to populate subscribe ds: TypeError: this._progressBar is null

Double checking now to make sure I didn't mess something up
(In reply to comment #9)
> I'm getting an error with this patch: failed to populate subscribe ds:
> TypeError: this._progressBar is null

I see this to, I'll investigate.

(In reply to comment #8)
> Maybe in the long run, search doesn't need to use the status bar at all. 
> clarkbw?
> 
> Right now, quick search uses the meteors to show it is searching, and it puts
> up a result count (or a no result message) in the status bar.  I have
> introduced the concept of 'thinking' on a per-tab basis, so the tab could do
> its own meteor thing, much like firefox does when a tab is loading.  We
> wouldn't need the global meteor then.  In terms of saying "I found 3 things" or
> "I found nothing", we could probably use our existing Unread/Total status bar
> entries instead, or be more direct about it.  The status bar may historically
> be used for such things in applications, but it's not intuitive unless you're
> used to it.

I think I'm starting to fail to see what this has to do directly with this bug. Certainly the send in background notifications are something we'd have had with send later, but just got disabled for a while. The other stuff is knew, but we need to tell the user about it anyway.

For the search case, we could always change the "Total: nnn" to "Results: aaa", except for the no results case, in which case we could put something on the display where the results are normally displayed saying "no results found". Then you wouldn't need the text string, just the progress.
Updated patch. Bug 489587 actually caused the errors that Bryan was seeing on the Search Messages and subscribe windows - the issue was that they were creating nsMsgStatusFeedback on js load, not at window load time (probably the reason we had ensureStatusFields function in there before).

I've moved those creations into the relevant places so that they get called in the onload of the window - i.e. after the document elements have been created.

I've also dropped two gStatusBar globals that were begin set up but not actually used for anything.

I've also changed a window.MsgStatusFeedback check to

"MsgStatusFeedback" in window

as I was getting complaints about window.MsgStatusFeedback being undefined - I've seen it before recently and this was the fix I had to do elsewhere.

Hence re-requesting review.
Attachment #374337 - Attachment is obsolete: true
Attachment #374453 - Flags: ui-review?(clarkbw)
Attachment #374453 - Flags: review?(bienvenu)
Attachment #374337 - Flags: ui-review?(clarkbw)
Comment on attachment 374453 [details] [diff] [review]
Hook process activities to the status bar v3

subscribe ui works now...
Attachment #374453 - Flags: review?(bienvenu) → review+
I think we've pretty much decided that bug 476487 (interactive status bar) isn't going to make Thunderbird 3. Therefore this bug is the half-way house. Adding to the blocking list.
Flags: blocking-thunderbird3+
Whiteboard: [has patch][needs review clarkbw]
Target Milestone: --- → Thunderbird 3.0b3
Whiteboard: [has patch][needs review clarkbw] → [b3ux][has patch][needs review clarkbw]
Whiteboard: [b3ux][has patch][needs review clarkbw] → [b3ux][m6][has patch][needs review clarkbw]
Comment on attachment 374453 [details] [diff] [review]
Hook process activities to the status bar v3

works for me
Attachment #374453 - Flags: ui-review?(clarkbw) → ui-review+
Checked in: http://hg.mozilla.org/comm-central/rev/1a60aac99d5b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.