Closed Bug 1720197 Opened 3 years ago Closed 3 years ago

Move Exchange autodiscover code into a JSM

Categories

(Thunderbird :: Account Manager, task)

Thunderbird 91

Tracking

(thunderbird_esr78 unaffected, thunderbird_esr91+ fixed, thunderbird91 affected)

RESOLVED FIXED
92 Branch
Tracking Status
thunderbird_esr78 --- unaffected
thunderbird_esr91 + fixed
thunderbird91 --- affected

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(1 file)

Bug 1717334 didn't do that for some reason, which introduced a possible race condition whereby Exchange autodiscover finds an HTTP redirect, waits 2 seconds in case another detection succeeds, but then finds that it can't cancel the timeout because TimeoutAbortable only understands Timer.jsm's timeouts and this is still using window.setTimeout.

Attached patch Proposed patchSplinter Review

I had to add extra parameters so that I could pass the confirmExchange function from the UI to ExchangeAutoDiscover.jsm (obviously the JSM isn't in a position to do any prompting itself).

Attachment #9230804 - Flags: review?(geoff)

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

r=BenB, but darktrojan should review as well

Attachment #9230804 - Flags: review+
Status: NEW → ASSIGNED
Attachment #9230804 - Flags: review?(geoff) → review+
Target Milestone: --- → 92 Branch

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/b714b6a3def3
Move Exchange autodiscovery into a JSM. r=BenB,darktrojan

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED

Do you intend to backport this to 91?

Flags: needinfo?(neil)

This was fixed in TB 92, ESR 91 uplift still not requested. Who is managing this?

Flags: needinfo?(mkmelin+mozilla)

It's really up to patch authors to request uplift.

Flags: needinfo?(neil)
Flags: needinfo?(mkmelin+mozilla)

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

[Approval Request Comment]
Regression caused by (bug #): semi-regression from bug 1717734
User impact if declined: potential UI confusion
Testing completed (on c-c, etc.): been on c-c and beta for a while
Risk to taking this patch (and alternatives if risky): should be fairly safe, but could do with manual testing after uplift

Attachment #9230804 - Flags: approval-comm-esr91?

(In reply to Magnus Melin [:mkmelin] from comment #7)

It's really up to patch authors to request uplift.

How do you manage situations where the patch author for one reason or another doesn't request uplift, but - unlike in this case - the patch fixes a regression?

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

[Triage Comment]
Approved for esr91

Attachment #9230804 - Flags: approval-comm-esr91? → approval-comm-esr91+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: