Include account name in status bar messages when sending/receiving/getting/downloading/fetching new News messages

RESOLVED FIXED in Thunderbird 35.0

Status

MailNews Core
Networking: NNTP
--
enhancement
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: aceman, Assigned: sshagarwal)

Tracking

(Blocks: 2 bugs)

Trunk
Thunderbird 35.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #66860 +++

When you download new mail messages the account name should be mentioned in the status line, e.g.:

"There are no new messages on the server"	should be:
"account name: There are no new messages on the server for the account"

"Connect: Host connected, sending login information..."		should be:
"account name: Connect: Host connected, sending login information for..."

"Downloading message x of y"	should be:
"account name: Downloading message x of y for"

Especially if you have more than one account whose messages get downloaded at startup the current status messages aren't clear enough.
(Assignee)

Comment 1

5 years ago
Created attachment 8340647 [details] [diff] [review]
Patch v1

Possible fix.
Covers the ones mentioned in the description.
Attachment #8340647 - Flags: feedback?(acelists)
(Reporter)

Comment 2

4 years ago
Comment on attachment 8340647 [details] [diff] [review]
Patch v1

Review of attachment 8340647 [details] [diff] [review]:
-----------------------------------------------------------------

Seems to work for me nicely. Can you again push the constructing of the status line into SetProgressStatus?
Attachment #8340647 - Flags: feedback?(acelists)
(Assignee)

Comment 3

4 years ago
Created attachment 8342879 [details] [diff] [review]
Patch v2

Okay, done!
And, this shows the account name associated. In case of news, aren't we interested in knowing the individual subscribed folders one is retrieving?
E.g. I have news.mozilla.org and that has "m.d.a.thunderbird" and "m.s.thunderbird" and others, so what would you like status messages to contain?
"news.mozilla.org..." or "m.d.a.thunderbird: ...."

Though, the downloading headers messages are formed this way:
"news.mozilla.org: downloading headers x of y for m.d.a.thunderbird"
Still, if we want all the status messages to mention the folder name, please let   me know.

Thanks.
Attachment #8340647 - Attachment is obsolete: true
Attachment #8342879 - Flags: feedback?(acelists)
(Assignee)

Comment 4

4 years ago
Created attachment 8342880 [details] [diff] [review]
Patch v2.1

Sorry, forgot to hg qref.
Attachment #8342879 - Attachment is obsolete: true
Attachment #8342879 - Flags: feedback?(acelists)
Attachment #8342880 - Flags: feedback?(acelists)
(Reporter)

Comment 5

4 years ago
Comment on attachment 8342880 [details] [diff] [review]
Patch v2.1

Review of attachment 8342880 [details] [diff] [review]:
-----------------------------------------------------------------

I would love to see both account name and newsgroup name in the status message. But you will need a UI review for that. It may be quite long, but the idea is that the user will need to read the message only if the download takes too long (or hangs).
Attachment #8342880 - Flags: feedback?(acelists) → feedback+
(Assignee)

Updated

4 years ago
Blocks: 946643
(Assignee)

Comment 6

4 years ago
(In reply to :aceman from comment #5)
> I would love to see both account name and newsgroup name in the status
> message. But you will need a UI review for that. It may be quite long, but
> the idea is that the user will need to read the message only if the download
> takes too long (or hangs).

So this can land and bug 946643 can be fixed after this? :)
(Reporter)

Comment 7

4 years ago
Yes, sure :) Get review from Neil and bwinton :)
(Assignee)

Comment 8

4 years ago
Comment on attachment 8342880 [details] [diff] [review]
Patch v2.1

Okay, thanks :)
Attachment #8342880 - Flags: ui-review?(bwinton)
Attachment #8342880 - Flags: review?(neil)
(Assignee)

Comment 9

4 years ago
Created attachment 8350626 [details]
Screenshot showing the status bar message.
Attachment #8350626 - Flags: ui-review?(bwinton)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
Comment on attachment 8342880 [details] [diff] [review]
Patch v2.1

ui-r=me, based on the screenshot at http://imgur.com/03xeDOW
Attachment #8342880 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Comment 11

4 years ago
Comment on attachment 8350626 [details]
Screenshot showing the status bar message.

Screenshot ui-reviewed by bwinton.
Attachment #8350626 - Flags: ui-review?(bwinton) → ui-review+
Comment on attachment 8342880 [details] [diff] [review]
Patch v2.1

>-nsNNTPNewsgroupList::SetProgressStatus(const PRUnichar *message)
>+nsNNTPNewsgroupList::SetProgressStatus(const nsString aMessage)
Don't change this, you need the const PRUnichar* for the params. (Although it's const char16_t* by now.)

>     nsCOMPtr <nsIMsgStatusFeedback> feedback;
>     mailnewsUrl->GetStatusFeedback(getter_AddRefs(feedback));
> 
>     if (feedback) {
>-      feedback->ShowStatusString(nsDependentString(message));
>+      // prepending the account name to the status message.
[It's a shame we don't have a request we can pass to nsMsgStatusFeedback::OnStatus to avoid duplicating code.]

>+      nsresult rv;
>+      nsCOMPtr <nsIMsgFolder> folder = do_QueryInterface(m_newsFolder, &rv);
>+      NS_ENSURE_SUCCESS_VOID(rv);
>+      nsCOMPtr <nsIMsgIncomingServer> server;
>+      rv = folder->GetServer(getter_AddRefs(server));
Can you not get the server directly from the mailnewsUrl?

>+      rv = sbs->CreateBundle("chrome://messenger/locale/messenger.properties",
>+                             getter_AddRefs(bundle));
MSGS_URL?

>+      const PRUnichar *params[] = { accountName.get(), aMessage.get() };
>+      bundle->FormatStringFromName(NS_LITERAL_STRING("statusMessage").get(),
>+                                   params, 2, getter_Copies(statusMessage));
We also use MOZ_UTF16() instead of NS_LITERAL_STRING().get() these days.
Attachment #8342880 - Flags: review?(neil) → review-
(Assignee)

Comment 13

4 years ago
Created attachment 8362595 [details] [diff] [review]
Patch v3.5
Attachment #8342880 - Attachment is obsolete: true
Attachment #8362595 - Flags: review?(neil)
Attachment #8362595 - Flags: feedback?(richard.marti)
Comment on attachment 8362595 [details] [diff] [review]
Patch v3.5

I only checked how it looks. I see the account name in front of the message.
Attachment #8362595 - Flags: feedback?(richard.marti) → feedback+
Created attachment 8362617 [details]
screenshot

Screenshot how it looks after polling. The only weird is, it says "No new messages on the server" but in screenshot you can see 4 new (unread) messages. On news I have only the headers downloaded. The body will be downloaded when I'm reading the messages. So the messages are still on the server. But this could be something for a new bug.

Comment 16

4 years ago
I don't know if that's what you saw, but new != unread. New is a message we didn't know about before.
Okay, I subscribed to a new group. After "Get Messages" the last message I saw was "No new messages on the server" but it disappeared almost immediately. The group shows now 47 new messages. And I would say, I didn't know about them before.
(In reply to aceman from comment #0)
> "Downloading message x of y"	should be:
> "account name: Downloading message x of y for"
> 
> Especially if you have more than one account whose messages get downloaded
> at startup the current status messages aren't clear enough.

Although, that's a bit of an edge case; you would need to be subscribed to the same newsgroup on two different servers.

Updated

4 years ago
Attachment #8362595 - Flags: review?(neil) → review+
(Assignee)

Comment 19

4 years ago
Thanks.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/bccf1119a409
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 35.0
You need to log in before you can comment on or make changes to this bug.