When adding an account, the outgoing server login is overwritten with the incoming server login

RESOLVED FIXED in Thunderbird 39.0

Status

Thunderbird
Account Manager
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: psychoslave, Assigned: aceman)

Tracking

31 Branch
Thunderbird 39.0

Thunderbird Tracking Flags

(thunderbird38+ fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141113113219

Steps to reproduce:

Add a account with a out server login which is different from the in server login.


Actual results:

The account is created with the in login server value in the out server login variable.


Expected results:

The account should be created with the out login server value in the out server login variable.
(Assignee)

Comment 1

3 years ago
Isn't this the same as bug 883670 ? But that one should be fixed in TB31. So please explain in more detail how you fill in the fields and what doesn't work.
(Reporter)

Updated

3 years ago
Summary: When adding a account, the out server login is overwritten with the in server login → When adding a account, the outgoing server login is overwritten with the ingoing server login
(Reporter)

Comment 2

3 years ago
Well, at least it's strongly related. When filling the "manual configuration" part of the wizard, the outgoing user name field is automaticaly copied when you change the incoming user name field. That's fine, less typing when they do match. The problem is that when you change the outgoing user name and valid the form, it will save the configuration with the incoming username. It also overwrite the outgoing user name with the incoming username when you "(re-)test" the configuration.

Should I add this details in the bug you pointed and reopen it (if a newcomer like me have such a privilege)?
(Reporter)

Updated

3 years ago
Summary: When adding a account, the outgoing server login is overwritten with the ingoing server login → When adding a account, the outgoing server login is overwritten with the incoming server login
(Reporter)

Updated

3 years ago
Summary: When adding a account, the outgoing server login is overwritten with the incoming server login → When adding an account, the outgoing server login is overwritten with the incoming server login
(Assignee)

Comment 3

3 years ago
OK, I can reproduce the problem.
The feature was implemented in bug 883670. Let's keep the bug here to fix what doesn't work in the feature. The process is to not reopen a bug that has a patch checked into TB31, when we are now at TB38, to ease tracking changes.
Assignee: nobody → acelists
Blocks: 883670
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 4

3 years ago
Created attachment 8559395 [details] [diff] [review]
1128872.patch

This seems to work for me.
The line added back in https://bugzilla.mozilla.org/show_bug.cgi?id=883670#c62 overwrote the user filled in outgoing username. So I need to make it conditional.
Attachment #8559395 - Flags: review?(mkmelin+mozilla)
Attachment #8559395 - Flags: review?(ben.bucksch)
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED

Comment 5

3 years ago
Comment on attachment 8559395 [details] [diff] [review]
1128872.patch

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

LGTM, r=mkmelin

::: mailnews/base/prefs/content/accountcreation/accountConfig.js
@@ +61,5 @@
>    // email address domains for which this config is applicable
>    // { Array of Strings }
>    domains : null,
>  
> +  sameInOutUsernames : true,

add documentation (even if it is slightly self documenting)
Attachment #8559395 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 6

3 years ago
Created attachment 8560663 [details] [diff] [review]
1128872.patch v1.1

Thanks.
Attachment #8559395 - Attachment is obsolete: true
Attachment #8559395 - Flags: review?(ben.bucksch)
Attachment #8560663 - Flags: review?(ben.bucksch)

Comment 7

3 years ago
Logic looks good to me.
Just keep the sameInOutUsernames as member of the UI (EmailConfigWizard), not part of the |config|. It's a UI thing.

Updated

3 years ago
Attachment #8560663 - Flags: review?(ben.bucksch) → review-
(Assignee)

Comment 8

3 years ago
Created attachment 8574854 [details] [diff] [review]
patch v2

Ok, I have looked at the test better why it expects outgoing_username=incoming_username. Now it seems to me the test is wrong.
The autoconfig file for the example.com domain contains this:
<incomingServer type="pop3">
  <username>%EMAILLOCALPART%</username>
</incomingServer>
<outgoingServer type="smtp">
  <username>%EMAILADDRESS%</username>
</outgoingServer>

So it seems correct to me that the usernames the wizard generates differ. incoming=username, outgoing=username@domain.

So the line doing outgoing=incoming is no longer needed (originally wanted to be removed in bug  883670), because outgoing is already set sooner in getUserConfig().

Ben, you know more about the config files, can you look at this theory?
Attachment #8560663 - Attachment is obsolete: true
Attachment #8574854 - Flags: review?(ben.bucksch)

Comment 9

3 years ago
> The autoconfig file for the example.com domain contains this:
> <incomingServer type="pop3">
>   <username>%EMAILLOCALPART%</username>
> </incomingServer>
> <outgoingServer type="smtp">
>   <username>%EMAILADDRESS%</username>
> </outgoingServer>

If the test runs with this config file, and the test expects incoming and outgoing account name to be the same, then the test is clearly wrong, yes.
(Assignee)

Comment 10

3 years ago
It appears to me this code loads that file (http://hg.mozilla.org/comm-central/file/30f695264e00/mail/test/mozmill/account/xml/example.com):
  // Set the pref to load a local autoconfig file.
  let pref_name = "mailnews.auto_config_url";
  let url = collector.addHttpResource("../account/xml", "autoconfig");
  Services.prefs.setCharPref(pref_name, url);

  // Force .com MIME-Type to text/xml
  collector.httpd.registerContentType("com", "text/xml");

And it is trying to create an account for username@example.com .
(Assignee)

Comment 11

3 years ago
Yes, editing that file affects the test.

Comment 12

3 years ago
Comment on attachment 8574854 [details] [diff] [review]
patch v2

I didn't test that the patch does the right thing, in all cases.

But the code makes sense to me. r+
Attachment #8574854 - Flags: review?(ben.bucksch) → review+

Comment 13

3 years ago
If the connection between the test and the config file is not already obvious, it would be good to add a comment at least in the test, maybe in both places, that hint at that connection. r+ for any such comments that you want to add.
(Assignee)

Comment 14

3 years ago
Created attachment 8575474 [details] [diff] [review]
patch v2.1

Thanks.
Attachment #8574854 - Attachment is obsolete: true
Attachment #8575474 - Flags: review?(mkmelin+mozilla)

Updated

3 years ago
tracking-thunderbird38: --- → +

Updated

3 years ago
Attachment #8575474 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 39.0

Comment 17

3 years ago
Backed out for test failures.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 19

3 years ago
Sorry, backing it out seemed to resolve the test failures locally, but it turns out my mozilla repo was out of date.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED

Updated

3 years ago
status-thunderbird38: --- → fixed
You need to log in before you can comment on or make changes to this bug.