Closed
Bug 953627
Opened 10 years ago
Closed 10 years ago
make auto-reconnect cancelable/stoppable
Categories
(Instantbird Graveyard :: Account manager, enhancement)
Instantbird Graveyard
Account manager
Tracking
(Not tracked)
RESOLVED
FIXED
0.2b1
People
(Reporter: florian, Assigned: romain)
References
Details
Attachments
(3 files, 1 obsolete file)
8.45 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
5.21 KB,
patch
|
Details | Diff | Splinter Review | |
9.99 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 180 at 2009-04-24 20:47:00 UTC *** When an account is trying to reconnect, there should be a way to stop it. Adding a "(cancel)" button presented like a link, or a cross on the line of the account where the countdown is visible would be nice. We should also add this action to the context menu once we have it (bug 953626 (bio 179)).
Reporter | ||
Updated•10 years ago
|
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 180 at 2009-08-02 22:28:32 UTC *** Mass change of target milestone from 0.2a1 to 0.2 for bugs that we haven't fixed for alpha 1 but still think we are likely to fix before 0.2 final.
Target Milestone: 0.2a1 → 0.2
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 180 as attmnt 169 at 2009-08-05 20:37:00 UTC *** Adding a link "Cancel" at the end of line of the timer. focusable with tab, and active with enter.
Attachment #8351913 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → romain
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8351913 [details] [diff] [review] Patch V1 *** Original change on bio 180 attmnt 169 at 2009-08-05 21:08:51 UTC *** >diff --git a/instantbird/base/content/instantbird/account.xml b/instantbird/base/content/instantbird/account.xml >+ <method name="cancelReconnection"> >+ <body> >+ <![CDATA[ >+ if (this.reconnectUpdateInterval) { Indentation is not as in the other methods of this file. The other methods indent after opening the CDATA block. >+ if (this.hasAttribute("reconnectPending")) >+ this.removeAttribute("reconnectPending"); I think removeAttribute works (and just does nothing) if the attribute doesn't exist, so I don't think you need this test. > if (target.localName != "checkbox" && >+ target.getAttribute("anonid") != "cancel" && > (target.localName != "button" || > /^(dis)?connect$/.test(target.getAttribute("anonid")))) I added the regexp to get the anonid attribute only once. I'm not sure if that was wise, but now that it's done, it would be more logical to just change the regexp... > box.appendNotification(label, "autologinStatus", null, priority, [connectNowButton]); > }, >+ onCancelReconnection: function am_cancelReconnection(event) { >+ if (event.button == 0) { >+ this.accountList.selectedItem.cancelReconnection(); >+ event.stopPropagation(); >+ } >+ }, > processAutoLogin: function am_processAutoLogin() { Why did you place this method in the middle of 2 autologin-related methods? I think I'd have put it before the connect method, though I can't seem to get an explanation of why at this place and not after the delete method... >diff --git a/purple/purplexpcom/public/purpleIAccount.idl b/purple/purplexpcom/public/purpleIAccount.idl >+ void cancelReconnection(); Maybe add a comment before that method. Explain that it cancels the timer that automatically reconnects accounts that were disconnected with a non fatal error. >diff --git a/purple/purplexpcom/src/purpleAccount.cpp b/purple/purplexpcom/src/purpleAccount.cpp >--- a/purple/purplexpcom/src/purpleAccount.cpp >+++ b/purple/purplexpcom/src/purpleAccount.cpp >@@ -566,16 +566,28 @@ NS_IMETHODIMP purpleAccount::Disconnect( > PURPLE_ENSURE_INIT(mAccount); > LOG(("Attempting to disconnect %s\n", mAccount->username)); > > purple_account_set_enabled(mAccount, UI_ID, FALSE); > > return NS_OK; > } > >+/* void cancelReconnection(); */ >+NS_IMETHODIMP purpleAccount::CancelReconnection() >+{ >+ if (mTimer) { >+ mTimer->Cancel(); >+ mTimer = nsnull; >+ /* Canceled by user action */ I don't think this comment helps. Make it more useful or remove it. >+ mReconnectAttempts = 0; >+ } >+ return NS_OK; >+}
Attachment #8351913 -
Flags: review?(florian) → review-
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 180 as attmnt 170 at 2009-08-05 21:24:00 UTC *** Fixes the coding style nits described in Comment 3. Except the regex, it is not applicable here because the link is not a button.
Attachment #8351914 -
Flags: review?(florian)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8351913 [details] [diff] [review] Patch V1 *** Original change on bio 180 attmnt 169 at 2009-08-05 21:24:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351913 -
Attachment is obsolete: true
Reporter | ||
Comment 6•10 years ago
|
||
Comment on attachment 8351914 [details] [diff] [review] Patch V2 *** Original change on bio 180 attmnt 170 at 2009-08-06 01:11:23 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351914 -
Flags: review?(florian) → review+
Reporter | ||
Comment 7•10 years ago
|
||
*** Original post on bio 180 at 2009-08-06 01:44:27 UTC *** Comment on attachment 8351914 [details] [diff] [review] (bio-attmnt 170) Patch V2 >diff --git a/instantbird/base/content/instantbird/account.xml b/instantbird/base/content/instantbird/account.xml >--- a/instantbird/base/content/instantbird/account.xml >+++ b/instantbird/base/content/instantbird/account.xml >@@ -63,17 +63,23 @@ > <xul:label flex="1" xbl:inherits="value=alias" > class="accountAlias"/> > </xul:hbox> > <xul:label class="connecting" anonid="connecting" value="&account.connecting;"/> > <xul:label class="connected" value="&account.connected;"/> > <xul:label class="disconnecting" value="&account.disconnecting;"/> > <xul:label class="disconnected" value="&account.disconnected;"/> > <xul:description class="error" anonid="error"/> >- <xul:description class="error" anonid="reconnect"/> >+ <xul:hbox class="error timerBox"> >+ <xul:description anonid="reconnect"/> >+ <xul:spacer flex="1"/> >+ <xul:label class="text-link" anonid="cancel" >+ value="&account.cancelReconnection.label;" >+ onclick="gAccountManager.onCancelReconnection(event)"/> >+ </xul:hbox> I've just tried the patch, and I'm not sure aligning on the right the 'cancel' button is a good idea. I guess we will need to discuss that...
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 180 as attmnt 177 at 2009-08-07 23:00:00 UTC *** Only XPCOM and js here, we need to add the "button" in a context menu to come with bug 953626 (bio 179).
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 180 as attmnt 196 at 2009-08-16 10:32:00 UTC *** Fixing a few remaining bugs and adding an item to the context menu. The only point I was not sure for this patch is that when an account is no longer in "pendingReconnect" state, its reconnect description is hidden using display: none in CSS, but the text itself is still there (but of course it causes no visual problem).
Attachment #8351940 -
Flags: review?(florian)
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8351940 [details] [diff] [review] Patch V3 *** Original change on bio 180 attmnt 196 at 2009-08-17 01:56:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351940 -
Flags: review?(florian) → review+
Assignee | ||
Comment 11•10 years ago
|
||
*** Original post on bio 180 at 2009-08-17 08:01:16 UTC *** Additional changes were made before pushing: In purpleAccount.idl: - /* Cancel the timer that automatically reconnect the account that were + /* Cancel the timer that automatically reconnects the account that were and added a null check on accountList.selectedItem in disableCommandItems() to prevent this function to be called while the account manager is building (and thus have no item selected). Pushed as https://hg.instantbird.org/instantbird/rev/7f1a472e0705
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
Target Milestone: 0.2 → 0.2b1
You need to log in
before you can comment on or make changes to this bug.
Description
•