[autoconfig] POP3 Settings are used although IMAP is selected, for ISPs where POP3 is the default and IMAP is optional

RESOLVED FIXED in Thunderbird 7.0

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: nitrox202, Assigned: bwinton)

Tracking

unspecified
Thunderbird 7.0
x86
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(blocking-thunderbird5.0 beta2+, thunderbird5.0 beta2-fixed, thunderbird6 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110602 Firefox/7.0a1
Build Identifier: Thunderbird/5.0b1

Account creation wizard uses POP3 settings even when IMAP is selected for yahoo.com accounts.

Reproducible: Always

Steps to Reproduce:
To reproduce this issue, 
1.Install and Run thunderbird 5 beta 1 with fresh profile
2.In account creation wizard use a yahoo.com email account
3.After filling other details, click continue
4.Now Thunderbird will contact ISP Database and get the details
5.The radio box is selected (By default) as IMAP but the details are shown for POP3
If i continue to click create account, POP3 settings are used.
Note: If i click on IMAP radio box again, the settings are corrected.


Expected Results:  
The expected result is IMAP details should be shown and when i click create account IMAP details should be used.
We've disabled imap for yahoo as the yahoo infrastructure aren't ready for desktop client load on them. :-)
> uses POP3 settings even when IMAP is selected

That is a bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 549045
Summary: POP3 Settings are used when IMAP is selected in Add new mail account dialog for yahoo.com accounts → [autoconfig] POP3 Settings are used although IMAP is selected, for ISPs where POP3 is the default and IMAP is optional
blocking-thunderbird5.0: --- → ?
Assignee: nobody → bwinton
blocking-thunderbird5.0: ? → beta2+
Posted patch A patch to fix various TODOs. (obsolete) — Splinter Review
So, it looked like the bug was because we were setting the value of the radiogroup before we populated the configIncomings of the elements, which led to this.  And then we had another bug where we would never remove elements from the alternatives, so I fixed that, too.
Attachment #537845 - Flags: review?(ben.bucksch)
Turns out we couldn't filter out the objects because some of them were filled out.  So this patch fixes that.  :)
Attachment #537845 - Attachment is obsolete: true
Attachment #537845 - Flags: review?(ben.bucksch)
Attachment #537851 - Flags: review?(ben.bucksch)
So, if we leave the big block of code in the displayConfigResult function, then it calls onResultIMAPOrPOP3 (when changing e("result_imappop").value), which then recursively calls displayConfigResult again.  I don't think that we really want set the selected radio button from the callback when the user has set the radio button, so I've moved this code out of displayConfigResult, and into _foundConfig2, but I'm not sure that's the right thing to do.

We should chat about the design tomorrow on IRC, and see if this is okay, or if there's a better way to handle it…

Thanks,
Blake.
Attachment #537851 - Attachment is obsolete: true
Attachment #537851 - Flags: review?(ben.bucksch)
Attachment #537934 - Flags: review?(ben.bucksch)
Attachment #537851 - Attachment is obsolete: false
Comment on attachment 537934 [details] [diff] [review]
Further digging revealed more problems…  ;)

> I've moved this code out of displayConfigResult, and into _foundConfig2

Please leave it where it is, just break that silly cycle you described. (Thanks!)
Attachment #537934 - Attachment is obsolete: true
Attachment #537934 - Flags: review?(ben.bucksch)
Here we go.  Hopefully the last patch.  :)
Attachment #537851 - Attachment is obsolete: true
Attachment #538027 - Flags: review?(ben.bucksch)
Comment on attachment 538027 [details] [diff] [review]
Only call the handler on user action.

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

Thanks, Blake!

r=BenB with the 2 things fixed.

::: mailnews/base/prefs/content/accountcreation/emailWizard.js
@@ +847,5 @@
> +          config.incoming;
> +      e("result_imappop").onselect = null
> +      e("result_imappop").value =
> +          config.incoming.type == "imap" ? 1 : 2;
> +      e("result_imappop").onselect = this.onResultIMAPOrPOP3;

Remove this .onselect=... and the .onselect=null above.

You said on IRC: "Oh, detritus from an earlier version."

@@ +858,5 @@
>  
> +  // This function must only be called by user action, not by setting the
> +  // value or selectedItem or selectedIndex of the radiogroup!  This is why
> +  // we use the oncommand attribute of the radio elements instead of the
> +  // onselect attribute of the radiogroup!

Please make this a JavaDoc comment, compare the other functions
Attachment #538027 - Flags: review?(ben.bucksch) → review+
Carrying forward Ben's r+…

I was hoping to check this in, but then the tree started burning, so I'm posting this so that it doesn't get lost in the shuffle…

Thanks,
Blake.
Attachment #538027 - Attachment is obsolete: true
Attachment #538101 - Flags: review+
Keywords: checkin-needed
Attachment #538101 - Flags: approval-thunderbird5.0?
Attachment #538101 - Flags: review+
Committed as http://hg.mozilla.org/comm-central/rev/6d97c01e32a5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment #538101 - Flags: approval-thunderbird5.0?
Attachment #538101 - Flags: approval-thunderbird5.0+
Attachment #538101 - Flags: approval-comm-aurora?
Attachment #538101 - Flags: approval-comm-aurora? → approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.