Closed Bug 953572 Opened 10 years ago Closed 10 years ago

IB should try to reconnect while connecting and the proxy settings are changed

Categories

(Instantbird Graveyard :: Account manager, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: romain, Assigned: romain)

Details

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 126 at 2008-09-25 19:02:00 UTC ***

All is said in the summary :)
When an account is currently "connecting", and we change the proxy settings, Instantbird should try to disconnect / reconnect with the new settings.
Attached patch Patch V1 (obsolete) — Splinter Review
*** Original post on bio 126 as attmnt 149 at 2009-07-30 23:33:00 UTC ***

After investigations on how to do it simply, I came to this patch.

Works only in that cases:
- An account is currently connecting or disconnected (with an error, bad password, bad proxy, anything ...).
- Then Someone is editing this account's properties and clicks on "OK" (after changing a proxy, changing a password, or changing nothing at all, just clicks on "OK")
- Then a disconnection will be proceed on that account so that its state is "Not Connected", and right After a connection is performed so that the new parameters are used for the connection.


So, the user has not to wait or to click on "Connect" to have his/her account connected, it is done automatically given the criteria ahead.

At the beginning it was only for proxy purposes (when you change your location it is boring to change the proxy, click OK then OK then Connect, but it is simpler and more logical to extend it to all accounts parameters).

Tested extensively on GNU/Linux Ubuntu 9.06.
Attachment #8351893 - Flags: review?(florian)
Assignee: nobody → romain
Status: NEW → ASSIGNED
Comment on attachment 8351893 [details] [diff] [review]
Patch V1

*** Original change on bio 126 attmnt 149 at 2009-07-31 23:08:17 UTC ***

I agree that this should apply for all the settings that are likely to have an impact on the ability to successfully connect the account.

Stopping the currently in progress connection attempt when someone opens the properties dialog and clicks OK without changing anything is not acceptable though.

Looking at the code in the save method in account.js, it's trivial to know if the password has changed.
You can assume (even though that's not 100% right) that all protocol specific options have an impact on the connection so that part is easy too. The local alias has no impact.

For the proxies, I think you can compare 2 proxies by testing proxy1.key == proxy2.key.
Hmm... except that this won't work if you just updated the password of the proxy because in this case the proxy object is modified instead of a new one created.
Attachment #8351893 - Flags: review?(florian) → review-
Attached patch Patch V2 (obsolete) — Splinter Review
*** Original post on bio 126 as attmnt 157 at 2009-08-01 12:34:00 UTC ***

This patch adds a variable set to false at the beginning, and is true if anything that might affect connection is changed.
Attachment #8351901 - Flags: review?(florian)
Comment on attachment 8351893 [details] [diff] [review]
Patch V1

*** Original change on bio 126 attmnt 149 at 2009-08-01 12:34:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351893 - Attachment is obsolete: true
Comment on attachment 8351901 [details] [diff] [review]
Patch V2

*** Original change on bio 126 attmnt 157 at 2009-08-01 16:58:13 UTC ***

>diff --git a/instantbird/base/content/instantbird/account.js b/instantbird/base/content/instantbird/account.js

>-    this.account.proxyInfo = this.proxy;
>+    if (this.account.proxyInfo.key != this.proxy.key ||

>+        this.account.proxyInfo.password != this.proxy.password) {
If the proxy wasn't changed but only the password was updated, this test won't save you, because this.account.proxyInfo and this.proxy are probably the same. And if this.proxy.key is none or envvar, trying to get the password will probably cause a JS strict warning.


>+    if (connectionInfoHasChanged && (this.account.connecting ||
>+        (this.account.disconnected &&
>+         this.account.connectionErrorReason != Ci.purpleIAccount.NO_ERROR))) {
>+      this.account.disconnect();
>+      this.account.connect();
>+    }
>   },

Calling disconnect on an account that is already disconnected with an error doesn't seem like a good thing to do (though it will probably work or fail without doing any harm).
Attachment #8351901 - Flags: review?(florian) → review-
Attached patch Patch V3Splinter Review
*** Original post on bio 126 as attmnt 162 at 2009-08-04 17:24:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351906 - Flags: review?(florian)
Comment on attachment 8351901 [details] [diff] [review]
Patch V2

*** Original change on bio 126 attmnt 157 at 2009-08-04 17:24:02 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351901 - Attachment is obsolete: true
Comment on attachment 8351906 [details] [diff] [review]
Patch V3

*** Original change on bio 126 attmnt 162 at 2009-08-04 21:46:52 UTC ***

Looks good!
Attachment #8351906 - Flags: review?(florian) → review+
*** Original post on bio 126 at 2009-08-04 23:14:13 UTC ***

attachment 8351906 [details] [diff] [review] (bio-attmnt 162) pushed:
https://hg.instantbird.org/instantbird/rev/48057d3a732b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.2b1
You need to log in before you can comment on or make changes to this bug.