Closed Bug 678255 Opened 13 years ago Closed 13 years ago

Advanced dialog for POP3 account showing the IMAP dialog

Categories

(Thunderbird :: Account Manager, defect)

x86_64
Windows 7
defect
Not set
major

Tracking

(thunderbird8+ fixed, thunderbird9+ fixed)

RESOLVED FIXED
Thunderbird 10.0
Tracking Status
thunderbird8 + fixed
thunderbird9 + fixed

People

(Reporter: rkent, Assigned: ewong)

References

Details

(Keywords: regression)

Attachments

(2 files)

On current Trunk (but not Aurora), in the account manager if I select for a POP3 account Server Settings/Advanced, I get the IMAP version and not the POP3 version. I also cannot save any settings, so this means that the advanced dialog is non-functional for POP3 servers.
Different phenomenon in next trunk nightly build.
> Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110911 Thunderbird/9.0a1
Observed phenomena:
(i) Server Settings/Advanced of both POP3 account and IMAP account shows same settings; Mixture of setting for IMAP(upper half, starts with "IMAP server directory:") and setting for POP3(lower half, starts with "when downloading POP3 mail ...".
(ii) Not POP3 account only problem. IMAP setting such as "max cached conection" is also not saved. Shown as blank always, even after setting change via UI.
ewong, according to the hg it's you who touched the files in bug 621042. By looking at the patch and mxr-ing, it seems the forAccount entity is missing for Thunderbird. Could this be the root of this bug?
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #563687 - Flags: review?(philringnalda)
Attachment #563687 - Flags: review?(philringnalda) → review+
Keywords: checkin-needed
Depends on: 621042
Blocks: 621042
No longer depends on: 621042
This is changing the UI, and under the Thunderbird review rules we really should have UI-review for that (I suspect this small change is fine, but I want to verify anyway).
Keywords: checkin-needed
Attachment #563687 - Flags: ui-review?(bwinton)
Additionally, we need to deal with the regression on the Earlybird/Aurora channel. The issue here is that Thunderbird can't add the string (due to the string freeze on aurora). Therefore I can see two possible solutions:

1) Add a check in am-server-advanced.js to see if forAccount string exists before attempting to use it.
2) Backout the patch from bug 621042 apart from the string addition.

I'm happy for either to be done.
Comment on attachment 563687 [details] [diff] [review]
Added forAccount entity to the pref.properties.

Yeah, the string looks good to me.

Thanks,
Blake.
Attachment #563687 - Flags: ui-review?(bwinton) → ui-review+
(In reply to Mark Banner (:standard8) from comment #5)
> Additionally, we need to deal with the regression on the Earlybird/Aurora
> channel. The issue here is that Thunderbird can't add the string (due to the
> string freeze on aurora). Therefore I can see two possible solutions:
> 
> 1) Add a check in am-server-advanced.js to see if forAccount string exists
> before attempting to use it.
> 2) Backout the patch from bug 621042 apart from the string addition.
> 
> I'm happy for either to be done.

Out of curiosity, was I cc'ed just incase I feel SeaMonkey needs to track this? Or what.

By the conversation here, and code inspection it looks like this works fine in SeaMonkey (even though the patch that did it for SeaMonkey broke TB here).

Also, I'm ok with either option as well.
(In reply to Justin Wood (:Callek) from comment #7)
> (In reply to Mark Banner (:standard8) from comment #5)
> > Additionally, we need to deal with the regression on the Earlybird/Aurora
> > channel. The issue here is that Thunderbird can't add the string (due to the
> > string freeze on aurora). Therefore I can see two possible solutions:
> > 
> > 1) Add a check in am-server-advanced.js to see if forAccount string exists
> > before attempting to use it.
> > 2) Backout the patch from bug 621042 apart from the string addition.
> > 
> > I'm happy for either to be done.
> 
> Out of curiosity, was I cc'ed just incase I feel SeaMonkey needs to track
> this? Or what.

In case SM devs had opinions over backing out or adding an additional fix (I would probably tend to back out as it is easier).
Does this simply need landing?
As Blake is happy with it, this can be landed on trunk but not branches. We need to do one of comment 5 for branches. I'm tending on partial backing out as this isn't a critical function. I can get a patch up tomorrow.
Checked in on trunk: http://hg.mozilla.org/comm-central/rev/227eaa4b8ccc

Hence marking fixed. Will sort out the branches tomorrow.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 10.0
This is the proposed backout which will be applied to aurora and beta. The fix will then make it into gecko 10 based versions onwards.
Comment on attachment 567853 [details] [diff] [review]
Backout non string parts

rs=me.
Attachment #567853 - Flags: review+
(In reply to Mark Banner (:standard8) from comment #13)
> Created attachment 567853 [details] [diff] [review] [diff] [details] [review]
> Backout non string parts

Checked into aurora and beta:

http://hg.mozilla.org/releases/comm-aurora/rev/84b6bba97413
http://hg.mozilla.org/releases/comm-beta/rev/6ae3394e74d0
> Checked in on trunk: http://hg.mozilla.org/comm-central/rev/227eaa4b8ccc

Verified with next trunk nightly build.
> Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111018 Thunderbird/10.0a1
You need to log in before you can comment on or make changes to this bug.