Changing the status to available while an IRC account is in the disconnecting state doesn't reconnect it

RESOLVED FIXED in 1.4

Status

Chat Core
IRC
--
minor
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: aleth)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
*** Original post on bio 1846 at 2012-12-05 18:45:00 UTC ***

Typically happens when I use /offline to quickly disconnect all accounts and /back immediately after /offline to reconnect them all, when I know my network connections are dead because I've switched to a different wifi network.

In that case, IRC takes a few seconds to timeout; other accounts get marked as disconnected almost immediately, so they are already in the disconnected state when I type /back.
(Assignee)

Comment 1

4 years ago
Created attachment 8353914 [details] [diff] [review]
Patch

*** Original post on bio 1846 as attmnt 2152 at 2012-12-05 22:17:00 UTC ***

Set the reconnect timer if the status is changed from OFFLINE while an account is still disconnecting, so it will connect as soon as it has disconnected.
Attachment #8353914 - Flags: review?(florian)
(Assignee)

Comment 2

4 years ago
Created attachment 8353916 [details] [diff] [review]
Patch

*** Original post on bio 1846 as attmnt 2154 at 2012-12-05 23:01:00 UTC ***

The above patch fails if the disconnection takes longer than 1s (the first reconnection timer length). 

This approach simply connects as soon as the account is disconnected when appropriate.
Attachment #8353916 - Flags: review?(florian)
(Assignee)

Comment 3

4 years ago
Comment on attachment 8353914 [details] [diff] [review]
Patch

*** Original change on bio 1846 attmnt 2152 at 2012-12-05 23:01:04 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353914 - Attachment is obsolete: true
Attachment #8353914 - Flags: review?(florian)
(Assignee)

Updated

4 years ago
Assignee: nobody → aleth
Status: NEW → ASSIGNED
(Reporter)

Comment 4

4 years ago
Comment on attachment 8353916 [details] [diff] [review]
Patch

*** Original change on bio 1846 attmnt 2154 at 2012-12-05 23:03:22 UTC ***

Looks fine to me, thanks!
Attachment #8353916 - Flags: review?(florian) → review+
*** Original post on bio 1846 at 2012-12-05 23:36:54 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/24431396e7ec
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4
(Assignee)

Comment 6

4 years ago
Created attachment 8353925 [details] [diff] [review]
Patch to fix bustage

*** Original post on bio 1846 as attmnt 2163 at 2012-12-11 21:58:00 UTC ***

Check whether the disconnected account was actually connected without errors before attempting to reconnect. (This avoids e.g. immediate reconnection on status change when there was a reconnection timer present, and reconnection on status change when the account is incorrectly configured.)
Attachment #8353925 - Flags: review?(florian)
(Reporter)

Comment 7

4 years ago
Comment on attachment 8353925 [details] [diff] [review]
Patch to fix bustage

*** Original change on bio 1846 attmnt 2163 at 2012-12-12 07:17:50 UTC ***

>diff --git a/components/imAccounts.js b/components/imAccounts.js

>     else if (aTopic == "account-disconnected") {
>       this.connectionState = Ci.imIAccount.STATE_DISCONNECTED;
>       if (this._statusObserver &&
>+          this.prplAccount.connectionErrorReason == Ci.prplIAccount.NO_ERROR &&

Why this.prplAccount.connectionErrorReason and not just this.connectionErrorReason?

>@@ -625,17 +626,18 @@ imAccount.prototype = {

>           else if (statusType > Ci.imIStatusInfo.STATUS_OFFLINE &&
>-                   this.disconnected)
>+                   this.disconnected && (!this.prplAccount.connectionErrorReason ||

What's the !connectionErrorReason test for? It looks like a null check, but 0 is actually ERROR_NETWORK_ERROR.
Attachment #8353925 - Flags: review?(florian) → review-
(Assignee)

Comment 8

4 years ago
Created attachment 8353927 [details] [diff] [review]
Patch to fix reconnect timer bustage

*** Original post on bio 1846 as attmnt 2165 at 2012-12-12 18:45:00 UTC ***

>Why this.prplAccount.connectionErrorReason and not just
>this.connectionErrorReason?
I've now used this.connectionErrorReason in the status observer and kept this.prplAccount.connectionErrorReason in the account observer (for consistency).

>What's the !connectionErrorReason test for? It looks like a null check, but 0
>is actually ERROR_NETWORK_ERROR.
It was intended to check for 'undefined' but this is unnecessary due to the default value in the prototype.
Attachment #8353927 - Flags: review?(florian)
(Assignee)

Comment 9

4 years ago
Comment on attachment 8353925 [details] [diff] [review]
Patch to fix bustage

*** Original change on bio 1846 attmnt 2163 at 2012-12-12 18:45:49 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353925 - Attachment is obsolete: true
(Reporter)

Comment 10

4 years ago
Comment on attachment 8353927 [details] [diff] [review]
Patch to fix reconnect timer bustage

*** Original change on bio 1846 attmnt 2165 at 2012-12-14 23:03:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353927 - Flags: review?(florian) → review+
*** Original post on bio 1846 at 2012-12-14 23:31:47 UTC ***

attachment 8353927 [details] [diff] [review] (bio-attmnt 2165) checked in as http://hg.instantbird.org/instantbird/rev/16136295ea7b

Updated

4 years ago
Duplicate of this bug: 955289
(Reporter)

Updated

4 years ago
Depends on: 955376
You need to log in before you can comment on or make changes to this bug.