PriorityOrderAbortable is too successful
Categories
(Thunderbird :: Account Manager, defect)
Tracking
(thunderbird_esr6069+ fixed, thunderbird_esr6869+ fixed, thunderbird69 fixed, thunderbird70 fixed)
People
(Reporter: neil, Assigned: neil)
References
Details
Attachments
(1 file, 1 obsolete file)
1.88 KB,
patch
|
BenB
:
review+
jorgk-bmo
:
approval-comm-beta+
jorgk-bmo
:
approval-comm-esr60+
jorgk-bmo
:
approval-comm-esr68+
|
Details | Diff | Splinter Review |
PriorityOrderAbortable
wants to call successCallback
on all successful calls in priority order until it finds one for which successCallback
itself succeeds.
However, having found a successful call with a successful callback, it proceeds to call this callback again every time any of its other calls completes for any reason.
Assignee | ||
Comment 1•4 years ago
|
||
The variable that keeps track of which call's callback was successful is local to the observer, so we forget that we called it. Instead, once we know that a success callback succeeded, we shouldn't call the success callback again.
We don't have to store the successful call itself but I did just in case it could be of interest.
Assignee | ||
Comment 2•4 years ago
|
||
Comment on attachment 9084039 [details] [diff] [review] Proposed patch Ben gave me r+ over the phone as an interim measure but we have an idea for an alternative version.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Assignee | ||
Comment 4•4 years ago
|
||
Note that while testing these patches I noticed that they exacerbate a Trunk-only bug whereby the Exchange autodiscovery can continue after another method has already found a configuration. Without this patch, we refresh the UI after that discovery finishes, but these patches optimise away that refresh...
Assignee | ||
Comment 5•4 years ago
|
||
Comment on attachment 9085090 [details] [diff] [review] Alternative approach Ben decided not to go with this version as the other version was clearer.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d924fa692a42
Don't call successCallback again once it succeeds. r=BenB
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Comment on attachment 9084039 [details] [diff] [review] Proposed patch [Approval Request Comment] Regression caused by (bug #): Bug in new code added by 1500105 User impact if declined: Not visible to user, causes bug 1571772 to make unnecessary requests Testing completed (on c-c, etc.): Risk to taking this patch (and alternatives if risky):
Updated•4 years ago
|
Comment 8•4 years ago
|
||
TB 69 beta 4:
https://hg.mozilla.org/releases/comm-beta/rev/cdb50d9fdd906047708d2a84b7c6268b6d765c47
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 9•4 years ago
|
||
TB 68.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/c3317cbda62d9f7b6197302fe66acc784c214ff0
Updated•4 years ago
|
Comment 10•4 years ago
|
||
TB 60.9 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/27f74f35b9d37f1ee46632434df4bdd575cc8bfd
Updated•4 years ago
|
Description
•