Closed Bug 1598041 Opened 2 months ago Closed 2 months ago

Switching between IMAP and Exchange options hides error message when IMAP fails

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set

Tracking

(thunderbird_esr68 fixed, thunderbird71 fixed, thunderbird72 fixed)

RESOLVED FIXED
Thunderbird 72.0
Tracking Status
thunderbird_esr68 --- fixed
thunderbird71 --- fixed
thunderbird72 --- fixed

People

(Reporter: neil, Assigned: neil)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

If Thunderbird offers both IMAP and Exchange, then if you toggle between the two, and you don't yet have any add-ons installed, then the status area gets hidden, and no error message is displayed if your attempt to configure an IMAP account fails.

e("result_select_" + alt.type).configIncoming = alt;

I also got a e(...) is null error on this line (during my testing of bug 1592258), if I had options before and they are no longer available.

Attached patch Proposed patchSplinter Review

The other alternative would be to show the status-area when switching back to IMAP, but this way is slightly nicer UI.

Assignee: nobody → neil
Attachment #9110299 - Flags: review?(ben.bucksch)
Comment on attachment 9110299 [details] [diff] [review]
Proposed patch

r+

I concur for the same reasons. We should not hide the error message that might be there. It's also nice to see where the config came from.
Attachment #9110299 - Flags: review?(ben.bucksch) → review+

Tested and fixes the bug for me.
Please land.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4498a8428f41
Don't hide error message when offering addon r=BenB

Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 72.0

Thanks, Geoff :)

Comment on attachment 9110299 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): Exposed by bug 1571772, since that created a previously unavailable UI combination
User impact if declined: Confusing display in certain cases
Testing completed (on c-c, etc.): n/a
Risk to taking this patch (and alternatives if risky): No risk, removal of unnecessary code.
Attachment #9110299 - Flags: approval-comm-beta?
Comment on attachment 9110299 [details] [diff] [review]
Proposed patch

Do it!
Attachment #9110299 - Flags: approval-comm-beta? → approval-comm-beta+
Attached patch ESR portSplinter Review

Trivial 1-character change.

[Approval Request Comment]
Regression caused by (bug #): Exposed by bug 1571772, since that created a previously unavailable UI combination
User impact if declined: Confusing display in certain cases
Testing completed (on c-c, etc.): n/a
Risk to taking this patch (and alternatives if risky): No risk, removal of unnecessary code.

Attachment #9113737 - Flags: approval-comm-esr68?
Attachment #9113737 - Flags: approval-comm-esr68? → approval-comm-esr68+

Ben, Neil: please no cross-reviewing by you two on account setup. There's a feedback flag so please use that instead.
(Not that it matters much for this bug.)

You need to log in before you can comment on or make changes to this bug.