482489 - Feedback/Alert notifications need identity/connection information

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

At the moment the feedback/alert notifications only have the information that is in the error message, e.g. "Failed to connect to server xxx".

We should try and provide identity/connection/account information to allow some grouping/more information about where the feedback originated from.

Comment 1

[Mark Banner (:standard8)]

Attachment: Backend support

This builds on the new alerts system by replacing the nsIMsgWindow argument with an nsIMsgMailNewsUrl argument. This gives us the benefit that nsIMsgMailNewsUrl knows about the relevant nsIMsgIncomingServer as well as the nsIMsgWindow.

There should be a little bit of code size reduction in there as well as we're not getting the message window in multiple places, just where we need it.

Comment 2

[Mark Banner (:standard8)]

Attachment: Proposed UI

Proposed UI - effectively we will group errors/warnings underneath their account name - matching the account name in the folder pane.

This gives more context, especially in the case where a user may have multiple accounts on one server.

Not sure if we should prefix the grouping by "Account: " to give slightly more information as to what its actually referring to.

Comment 3

[Mark Banner (:standard8)]

Attachment: UI patch

Comment 4

[David :Bienvenu]

there may be cases where there is no url - for example, we may get an alert notification from an idle imap connection.  Or we may be doing some activity that doesn't have a url, like compacting folders, and want to put up an alert. So we might want a flavor that takes an nsIMsgWindow as well. Or some way to poke the alert w/o having a url...but the patch looks OK at first look.

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> there may be cases where there is no url - for example, we may get an alert
> notification from an idle imap connection.  Or we may be doing some activity
> that doesn't have a url, like compacting folders, and want to put up an alert.
> So we might want a flavor that takes an nsIMsgWindow as well. Or some way to
> poke the alert w/o having a url...but the patch looks OK at first look.

Note the url is (as the nsIMsgWindow was) an optional argument. Unless I've missed something, all the cases in which we currently call AlertUser we are getting a message window from the url (the nsImapProtocol ones call GetMsgWindow which gets it from the running url).

I'm happy to add an nsIMsgWindow as an extra argument (or perhaps a different function would be clearer), but I don't see the need for it in the code at the moment.

Comment 6

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

Comment on attachment 371307 [details]
Proposed UI

looks pretty good so far

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 371309 [details] [diff] [review]
UI patch

Bryan's happy with the look of the UI, so time for reviews on this.

Comment 8

[David :Bienvenu]

Right, I just meant that sometime in the future we may need the ability to do this without a nsIMsgMailNewsUrl - sometimes, when all else fails, we have ways of getting a msg window.

Comment 9

[Mark Banner (:standard8)]

Both patches checked in: http://hg.mozilla.org/comm-central/rev/23163b1cbecf