214854 - State in error message from server to which mail account it belongs to

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Anthony Howe]

User-Agent:       
Build Identifier: 2003080104

For the first time ever, I'm seeing a new message I presume is coming from the
MailNews part:

   Password expires in N days.

With multiple email accounts and wallet entries, I have no idea which account
this message is refering to.  Could you please append to the message some
indication of which email account will expire, since its not very useful as is.


Reproducible: Always

Steps to Reproduce:

Comment 1

[Frank Wein [:mcsmurf]]

Are you really sure that this message came from Mozilla? I can't remember that
I've ever seen such a message and also http://lxr.mozilla.org/mozilla/ doesn't
list such a term in the code.

Comment 2

[Anthony Howe]

I've been receiving it all day about every 10 minutes.  Since I use the Sky
Pilot theme, I know it comes from Mozilla, since all the dialogs with the
message appear in that theme. This is the first time I've seen it myself and
presume it was added only recently.

Comment 3

[Frank Wein [:mcsmurf]]

Can you download the latest nightly and see if this problem persists?

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

http://lxr.mozilla.org/mozilla/search?string=password+expires

This can't be coming from Mozilla since we don't have that string in the
codebase at all.  My guess is that this is an error posted by one of your
servers, and Mozilla just outputs it word-for-word.

Comment 5

[Anthony Howe]

You are indeed correct.  Mozilla is reporting an error message word for word
from two of my email accounts I figured out.  So its not a message from Mozilla
itself.

However, it would be useful if Mozilla prefixed or suffixed such dialog box
messages with server name from where the message came from.  The only way I
figured out which ones they were was to login to each IMAP and POP server using
telnet to see what they were saying.  This is a length process when you have
several mail accounts and requires a user to know the IMAP4 and POP3 protocols,
which defeats the purpose of using a mail client I think :)

Any chance of a fix for this for this dialog?

Comment 6

[Frank Wein [:mcsmurf]]

Confirming+found no dupe+changing summary

Comment 7

[Frank Wein [:mcsmurf]]

Reporter: What title had this dialog field (I just need this to track down
something)?

Comment 8

[Anthony Howe]

The title just said "Alert".  The message came from the IMAP response line with
[alert] attribute.

Comment 9

[Jo Hermans]

*** Bug 233530 has been marked as a duplicate of this bug. ***

Comment 10

[David :Bienvenu]

taking

Comment 11

[Constantine A. Murenin]

David, this idea was also mentioned as a second-thought in bug #66860. Not
marking this bug as a duplicate, because the fix for Alert dialogue is much more
useful than the fix for the status-line messages. 

Comment 12

[Jo Hermans]

*** Bug 262061 has been marked as a duplicate of this bug. ***

Comment 13

[chris hofmann]

not likely that we are going to have time to get this for 1.0.   if a patch
appears renominate.

Comment 14

[James B. Higgs]

Hello,

Maybe this is slightly off-topic, but sometimes I get messages from my mail
server telling me that my Inbox has reached the High Water Mark (a percentage
I have set myself as the level that I want to be warned that my Inbox is
getting full) or even that a message has been bounced. These messages
apparently have a null Date field, which apparently Mozilla interprets as
"12/31/69 7:00 PM", which means it gets filed way up at the top of my Inbox
where I'll never see it unless I deliberately check up there. Is this too
off-topic to address here, or do I need to go back to bug-hunting?

Thanx,
Jim H. (aka CuriousJ)

Comment 15

[:aceman]

This seems to be about alerts from IMAP servers. Maybe this just needs update of the alert dialog function, similarly to bug 221592 for POP3.
Suyash, can you look at this?

Comment 16

[Suyash Agarwal (:sshagarwal)]

Attachment: Partial Patch_a

This is a partial patch with some useless duplication that can/will be removed if not used till the end.
This is probably to check if the Patch is heading in the right direction and if yes, what else is required, and if no, what needs fixing.

Thanks.

Comment 17

[:aceman]

Comment on attachment 804476 [details] [diff] [review]
Partial Patch_a

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

I can confirm the status bar for IMAP strings now contains the account name prefixed. I you think it needs some more visible delimiter. ": " make the account name still blend with the message, at least in my eyes. But we have UX for that :)

Comment 18

[Suyash Agarwal (:sshagarwal)]

So, can you please put up a screenshot showing that?
That will be easier to look at, rather than the patch (I think).

Because I can't reproduce any error, my TB isn't connecting to the internet (port issues).

Thanks.

Comment 19

[:aceman]

Attachment: IMAP status bar after the patch

Comment 20

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 805048 [details]
IMAP status bar after the patch

That seems reasonable, for what it's worth, and assuming we don't want to try and inject the mail server name into the message itself (like "Checking mail server ([e-mail]) capabilities…")…  ;)

ui-r=me for this piece.  Feedback coming later.

Later,
Blake.

Comment 21

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch_a

The patch has f+ and ui-r+.
Also, please let me know if I have missed any messages/alerts while modifying.

Thanks.

Comment 22

[[e-mail]]

Comment on attachment 808314 [details] [diff] [review]
Patch_a

>+    accountName.Append(progressMsg);
>+    progressMsg.Assign(accountName);
I don't like this, it's not very localisable. Which messages does this affect?
(It's also in the wrong place in the method.)

Comment 23

[Suyash Agarwal (:sshagarwal)]

(In reply to [e-mail] from comment #22)
> Comment on attachment 808314 [details] [diff] [review]
> Patch_a
> 
> >+    accountName.Append(progressMsg);
> >+    progressMsg.Assign(accountName);
> I don't like this, it's not very localisable. Which messages does this
> affect?
> (It's also in the wrong place in the method.)

Ya, I forgot about the localisation issue :(
But why is this at the wrong place?

So, if I create another string Id, and then pass this message in it and form the display string, is that acceptable?

Comment 24

[Suyash Agarwal (:sshagarwal)]

By the message to be passed, I mean progressMsg and the display string returned refers to the progressMsg and accountName together forming a string.

And also then, ":" isn't localisable, so that will also be covered in this part.

Comment 25

[Suyash Agarwal (:sshagarwal)]

I think this affects all the progress status bar messages, by prefixing the account name to all the messages.

Comment 26

[:aceman]

(In reply to Suyash Agarwal (:sshagarwal) away till Oct 5 from comment #23)
> So, if I create another string Id, and then pass this message in it and form
> the display string, is that acceptable?

Yes you create a new string:
statusMessage=$1S : $2S
and then in the dialog code do bundle.getFormattedString("statusFormat",[accountName,progressMsg]).

Comment 27

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch_a v2

Done!

Comment 28

[:aceman]

Comment on attachment 808459 [details] [diff] [review]
Patch_a v2

Yes:) That also allows to trivially change "$1: $2" to "[$1] $2" or anything else in the future if anybody feels like that :)

Comment 29

[[e-mail]]

Comment on attachment 808459 [details] [diff] [review]
Patch_a v2

>-  nsString progressMsg;
>+  nsString progressMsg, progressString;
> 
>   nsCOMPtr<nsIMsgIncomingServer> server;
>   nsresult rv = GetServer(getter_AddRefs(server));
>+  nsString accountName;
Inconsistent - you put one new string on an existing declaration but another one on its own.

>+    const PRUnichar* params[] = { accountName.get(),
>+                                  progressMsg.get() };
I never got around to answering your question about the code being in the wrong place. It needs to go after the processing of the extra info. (This includes fetching the account name.)

Comment 30

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch_a v2 modified

Comment 31

[[e-mail]]

Comment on attachment 810445 [details] [diff] [review]
Patch_a v2 modified

>+imapStatusMessage=%1$S: %2$S
...

>+      progressMsg.Assign(progressString);
>+
>       DisplayStatusMsg(imapUrl, progressMsg);
Don't bother assigning it, just pass progressString directly to DisplayStatusMsg.

>+imapStatusMessage=$1S: $2S
Suite version of this string is wrong. r=me with that fixed.

Comment 32

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch_a v2 corrected

Thanks.

Comment 33

[Philip Chee]

Comment on attachment 811968 [details] [diff] [review]
Patch_a v2 corrected

Tested on a couple of IMAP accounts. Status bar progress messages now include the name of the account. f=me

(In reply to [e-mail] from comment #29)
>>-  nsString progressMsg;
>>+  nsString progressMsg, progressString;
>> 
>>   nsCOMPtr<nsIMsgIncomingServer> server;
>>   nsresult rv = GetServer(getter_AddRefs(server));
>>+  nsString accountName;
> Inconsistent - you put one new string on an existing declaration but another
> one on its own.

Nitpick. There is still one place where you did:
>+        nsString dialogTitle, accountName;

Comment 34

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v2

Thanks :Ratty, made the change.

I request to keep this bug open for a while so that I can take into account any alerts/errors/status messages that still don't include the account name.

Thanks.

Comment 35

[Suyash Agarwal (:sshagarwal)]

Changing product from Seamonkey to Mailnews core as the changes are made in Thunderbird too.
Please correct me if this isn't correct.

Thanks.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/0d3191dc09ae