Closed Bug 661795 Opened 10 years ago Closed 10 years ago
[autoconfig] POP3 Settings are used although IMAP is selected, for ISPs where POP3 is the default and IMAP is optional
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
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
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.
Turns out we couldn't filter out the objects because some of them were filled out. So this patch fixes that. :)
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.
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!)
Here we go. Hopefully the last patch. :)
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.
Committed as http://hg.mozilla.org/comm-central/rev/6d97c01e32a5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Checked into comm-miramar: http://hg.mozilla.org/releases/comm-miramar/rev/73904271e5da
Attachment #538101 - Flags: approval-comm-aurora? → approval-comm-aurora+
Checked into aurora: http://hg.mozilla.org/releases/comm-aurora/rev/45ba9b39b15c
You need to log in before you can comment on or make changes to this bug.