Feedback/Alert notifications need identity/connection information

RESOLVED FIXED in Thunderbird 3.0b3

Status

MailNews Core
Backend
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 3.0b3
Bug Flags:
blocking-thunderbird3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [b3ux][m3])

Attachments

(3 attachments)

(Assignee)

Description

9 years ago
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.
Flags: blocking-thunderbird3+
(Assignee)

Updated

9 years ago
Whiteboard: [b3ux]
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

9 years ago
Whiteboard: [b3ux] → [b3ux][m2]
(Assignee)

Updated

9 years ago
Whiteboard: [b3ux][m2] → [b3ux][m3]
(Assignee)

Comment 1

9 years ago
Created attachment 371304 [details] [diff] [review]
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.
Attachment #371304 - Flags: superreview?(bienvenu)
Attachment #371304 - Flags: review?(bienvenu)
(Assignee)

Comment 2

9 years ago
Created attachment 371307 [details]
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.
Attachment #371307 - Flags: ui-review?(clarkbw)
(Assignee)

Updated

9 years ago
Attachment #371307 - Attachment description: UI patch → Proposed UI
(Assignee)

Comment 3

9 years ago
Created attachment 371309 [details] [diff] [review]
UI patch

Comment 4

9 years ago
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.
(Assignee)

Comment 5

9 years ago
(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.
(Assignee)

Updated

9 years ago
Whiteboard: [b3ux][m3] → [b3ux][m3][needs review bienvenu,clarkbw]
Attachment #371307 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 371307 [details]
Proposed UI

looks pretty good so far
(Assignee)

Comment 7

9 years ago
Comment on attachment 371309 [details] [diff] [review]
UI patch

Bryan's happy with the look of the UI, so time for reviews on this.
Attachment #371309 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Whiteboard: [b3ux][m3][needs review bienvenu,clarkbw] → [b3ux][m3][needs review bienvenu]

Updated

9 years ago
Attachment #371309 - Flags: review?(bienvenu) → review+

Updated

9 years ago
Attachment #371304 - Flags: superreview?(bienvenu)
Attachment #371304 - Flags: superreview+
Attachment #371304 - Flags: review?(bienvenu)
Attachment #371304 - Flags: review+

Comment 8

9 years ago
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.
Whiteboard: [b3ux][m3][needs review bienvenu] → [b3ux][m3]
(Assignee)

Comment 9

9 years ago
Both patches checked in: http://hg.mozilla.org/comm-central/rev/23163b1cbecf
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.