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

RESOLVED FIXED in Thunderbird 27.0

Status

MailNews Core
Backend
RESOLVED FIXED
15 years ago
5 years ago

People

(Reporter: Anthony Howe, Assigned: sshagarwal)

Tracking

(Blocks: 1 bug, {polish})

Trunk
Thunderbird 27.0
polish
Bug Flags:
blocking-aviary1.0 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

15 years ago
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

15 years ago
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.
(Reporter)

Comment 2

15 years ago
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

15 years ago
Can you download the latest nightly and see if this problem persists?
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.
(Reporter)

Comment 5

15 years ago
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

15 years ago
Confirming+found no dupe+changing summary
Status: UNCONFIRMED → NEW
Component: Account Manager → Mail Window Front End
Ever confirmed: true
QA Contact: nbaca → esther
Summary: "Password expires in N days" does not state which account. → State in error message from server to which mail account it belongs to

Comment 7

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

Comment 8

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

Comment 9

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

Comment 10

15 years ago
taking
Assignee: sspitzer → bienvenu
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. 
Depends on: 66860
Keywords: polish
OS: Windows XP → All
Hardware: PC → All

Comment 12

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

Updated

14 years ago
Flags: blocking-aviary1.0?

Comment 13

14 years ago
not likely that we are going to have time to get this for 1.0.   if a patch
appears renominate.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Product: Browser → Seamonkey

Comment 14

12 years ago
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)
Assignee: bienvenu → nobody
QA Contact: esther → message-display

Comment 15

5 years ago
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?
Blocks: 66860
No longer depends on: 66860
(Assignee)

Comment 16

5 years ago
Created attachment 804476 [details] [diff] [review]
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.
Attachment #804476 - Flags: feedback?(acelists)

Comment 17

5 years ago
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 :)
Attachment #804476 - Flags: feedback?(bwinton)
Attachment #804476 - Flags: feedback?(acelists)
Attachment #804476 - Flags: feedback+
(Assignee)

Comment 18

5 years ago
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.
Flags: needinfo?(acelists)

Comment 19

5 years ago
Created attachment 805048 [details]
IMAP status bar after the patch
Attachment #805048 - Flags: ui-review?(bwinton)
Flags: needinfo?(acelists)
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 (ace@mail.com) capabilities…")…  ;)

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

Later,
Blake.
Attachment #805048 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Updated

5 years ago
Assignee: nobody → syshagarwal
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 21

5 years ago
Created attachment 808314 [details] [diff] [review]
Patch_a

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

Thanks.
Attachment #804476 - Attachment is obsolete: true
Attachment #804476 - Flags: feedback?(bwinton)
Attachment #808314 - Flags: ui-review+
Attachment #808314 - Flags: review?(neil)
Attachment #808314 - Flags: feedback+
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.)
(Assignee)

Comment 23

5 years ago
(In reply to neil@parkwaycc.co.uk 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?
(Assignee)

Comment 24

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

Comment 25

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

Comment 26

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

Comment 27

5 years ago
Created attachment 808459 [details] [diff] [review]
Patch_a v2

Done!
Attachment #808314 - Attachment is obsolete: true
Attachment #808314 - Flags: review?(neil)
Attachment #808459 - Flags: review?(neil)

Comment 28

5 years ago
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 :)
Attachment #808459 - Flags: feedback+
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.)
Attachment #808459 - Flags: review?(neil) → review-
(Assignee)

Comment 30

5 years ago
Created attachment 810445 [details] [diff] [review]
Patch_a v2 modified
Attachment #808459 - Attachment is obsolete: true
Attachment #810445 - Flags: review?(neil)
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.
Attachment #810445 - Flags: review?(neil) → review+
(Assignee)

Comment 32

5 years ago
Created attachment 811968 [details] [diff] [review]
Patch_a v2 corrected

Thanks.
Attachment #810445 - Attachment is obsolete: true

Comment 33

5 years ago
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 neil@parkwaycc.co.uk 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;
Attachment #811968 - Flags: feedback+
(Assignee)

Comment 34

5 years ago
Created attachment 812013 [details] [diff] [review]
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.
Attachment #811968 - Attachment is obsolete: true
Attachment #812013 - Flags: review+
(Assignee)

Updated

5 years ago
Component: MailNews: Message Display → Backend
Keywords: checkin-needed
Product: SeaMonkey → MailNews Core
(Assignee)

Comment 35

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

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