203099 - Asked for password, but doesn't ask for username if username not provided in the prefs - "message could not be sent ... for an unknown reason"

Status: RESOLVED INCOMPLETE

(MailNews Core :: Networking: SMTP, defect)

Description

[Christian Eyrich]

1. Configure a SMTP server, check "Use name and password" but don't enter a
Username.
2. Send a mail through this server.

Actual Results:
It asks for password but not for username and fails sending

Expected Results:
It asks for username and password and sending succeeds

Comment 1

[Christian Eyrich]

Oh, I forgot: Trunk nightly 2003042222 on Linux.

There's some curious code in nsSmtpProtocol::AuthLoginUsername and
nsSmtpProtocol::GetUsernamePassword.

It tries to get the username via smtpServer->GetUsername(getter_Copies(username)).
If this fails it tries to get the password via smtpServer->GetPassword(aPassword). 

If this works, it tries to get the username via smtpServer->GetUsername(aUsername).

If getting the password or the second try for getting username fails, it prompts
the user for password.

That's all. And it looks kind of stupid to me.

Comment 2

[Christian Eyrich]

-> to me

Comment 3

[Christian Eyrich]

Attachment: proposed patch

This patch changes GetUsernamePassword() to really get username and password.
It indirectly uses GetUsernamePasswordWithUI() over the new function
PromptForUsernamePassword().

This is all analogue to GetPassword/PromptPassword/GetPasswordWithUI, although
not few code is duplicate now. I could also merge GetUsernamePassword with
GetPassword and incorporate PromptForPassword/PromptForUsernamePassword.
That would result in one a little bigger function but less code overall.


What I'm not sure about is if GetUsernamePasswordWithUI() in nsSmtpServer
should assign the username get permanently. If the user didn't enter a username
in the prefs, we could assume he knew what he did.
That would make it possible to use one SMTP server for different accounts where
the server requires different username/password.


On this way I also changed the strings for NS_SMTP_PASSWORD_PROMPT and
POP3_ENTER_PASSWORD_PROMPT to 
Enter your password for %1$s logging into %2$s:
instead of
Enter your password for %1$s@%2$s:
because the old one fooled many users (see bug 90507, esp. comment 22).

Any comments are welcome.

Comment 4

[David :Bienvenu]

-    if (*aPassword && **aPassword)
+    if (**aPassword && *aPassword)
this will crash if *aPassword is null - I think it was the other way around for
that reason.

same thing here:
+    if (**aUsername && *aUsername)
+    {

Re your comments, they sound reasonable. I'm not sure if there was some reason
for the way things are. I can try to look through the checkin logs and see.

Comment 5

[Christian Eyrich]

> +    if (**aPassword && *aPassword)
> this will crash if *aPassword is null - I think it was the other way around for
> that reason.

Aua, anybody seen my brain? A crash if the pointers pointer is null was what I
wanted to avoid, thought it was wrong as it was. But you're right.

Comment 6

[Christian Eyrich]

Attachment: patch v2

Changed the wrong pointer-test-changes back.

Comment 7

[WADA:World Anti-bad-Duping Agency]

When patch for "dialog format correction" will be available?
You can find many questions & "Bug Reports" due to this bug at Mozillazine
Thunderbird forums, especialy in "Thunderbird Bug forum".
Dialog correction for POP server case(Bug 212937) has already been done.
Should we open another bug for "dialog format correction for SMTP server" only?

Comment 8

[Christian Eyrich]

WADA, due to some problems on fixing the original issue of this bug, I've done a
separate patch to change the dialog format.
I posted it to bug 90507 (where it belongs) and it has been checked in on
2003-10-21. So you should be able to see the effect in the latest trunk builds.

Comment 9

[Nikolay Shopik]

This is still exist in 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080831032002 Shredder/3.0b1pre

Comment 10

[Wayne Mery (:wsmwk)]

Christian, is more info needed from Nikolay?   
Do we have a regression?
Or is what's left still related to bug 90507?

Comment 11

[Wayne Mery (:wsmwk)]

Rod, does this address any of bug 426185?

Comment 12

[Rod Whiteley]

If the username is not set permanently from the prompt, then users will still complain that Thunderbird cannot remember their username even though they can see it in Account Settings.

If the username *is* set permanently from the prompt, that's crazy UI design -- to have a required field that can be left blank, and then prompt for the value at some later time in a separate dialog.

So I do not think that this approach is a useful one.

Comment 13

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 124167 [details] [diff] [review]
patch v2

Patch has bitrotted. Christian, up for a new patch? :)

$ patch -p0 --dry-run < ~/Desktop/p203099.diff 
patching file nsSmtpProtocol.cpp
Hunk #1 FAILED at 1667.
Hunk #2 FAILED at 1682.
Hunk #3 FAILED at 1702.
Hunk #4 FAILED at 1723.
Hunk #5 FAILED at 1767.
5 out of 5 hunks FAILED -- saving rejects to file nsSmtpProtocol.cpp.rej
patching file nsSmtpProtocol.h
Hunk #1 FAILED at 245.
1 out of 1 hunk FAILED -- saving rejects to file nsSmtpProtocol.h.rej
can't find file to patch at input line 184
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: nsMsgComposeStringBundle.h
|===================================================================
|RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgComposeStringBundle.h,v
|retrieving revision 1.37
|diff -u -w -r1.37 nsMsgComposeStringBundle.h
|--- nsMsgComposeStringBundle.h 2 Dec 2002 23:43:20 -0000       1.37
|+++ nsMsgComposeStringBundle.h 9 May 2003 10:02:57 -0000
--------------------------
File to patch:

Comment 14

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 15

[Wayne Mery (:wsmwk)]

Seems improbable that this would still exist.

bouvjaga, what do you think?

Comment 16

[buovjaga]

Still repro.

Went to Account settings, SMTP, removed my username - it let me do it! For IMAP, it does not let me remove it, it complains and just puts it back.

Then, when trying to send an email, it does not ask for username and the sending fails.

Arch Linux
TB 60.6.1

Comment 17

[Wayne Mery (:wsmwk)]

Version 91 has all new smtp backend code. If you can still reproduce this issue, please file a new bug report https://bugzilla.mozilla.org/enter_bug.cgi?product=MailNews%20Core&component=Networking%3A%20SMTP