Closed Bug 954930 Opened 10 years ago Closed 10 years ago

When starting with necko offline, connect accounts as soon as the network is back. Also avoid connecting accounts when starting with "-status offline" on the command line.

Categories

(Chat Core :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

References

Details

(Whiteboard: [1.2-wanted])

Attachments

(1 file, 5 obsolete files)

*** Original post on bio 1497 at 2012-06-08 17:09:00 UTC ***

The status appears as offline when using this command line parameter, but accounts marked as "connect automatically" connect, and autojoin IRC channels get joined. You can send messages to them too...
*** Original post on bio 1497 at 2012-06-08 17:12:43 UTC ***

Cf http://mxr.mozilla.org/comm-central/source/chat/components/src/imCore.js#271
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1497 as attmnt 1575 at 2012-06-08 22:26:00 UTC ***

As discussed on IRC, this will require changes (including new strings) in the account manager to make the behaviour transparent.
Attachment #8353330 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1497 as attmnt 1576 at 2012-06-08 22:37:00 UTC ***

Fixed missing }
Attachment #8353331 - Flags: review?(florian)
Comment on attachment 8353330 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1575 at 2012-06-08 22:37:27 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353330 - Attachment is obsolete: true
Attachment #8353330 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
*** Original post on bio 1497 at 2012-06-08 22:52:35 UTC ***

This patch will introduce the following changed behaviour: 

When connect() is called, the status observer is always registered, but the account is only connected if the user status and the network is online (fixing this bug). When the user later changes his status to online and/or the network becomes available, the account will connect.

Similarly, disconnect() can now be called even when offline (and the account is already disconnected). In that case there will be no reconnection attempts when online again.

The account manager should be changed to match:
- Ci.imIAccountsService.AUTOLOGIN_START_OFFLINE is obsolete
- need another new string in the account manager to distinguish between offline status, and when necko is offline
- the "connect"/"disconnect" buttons should remain enabled in the account manager even when the status is OFFLINE, and clicking "connect" while offline would just change the message from "Not connected" to "Will be connected when leaving the offline status"
Depends on: 954932
Blocks: 954651, 954771
No longer depends on: 954932
Depends on: 954932
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1497 as attmnt 1578 at 2012-06-08 23:07:00 UTC ***

Non-behaviour changing parts of the last patch now in bug 954932 (bio 1499).
Attachment #8353334 - Flags: review?(florian)
Comment on attachment 8353331 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1576 at 2012-06-08 23:07:53 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353331 - Attachment is obsolete: true
Attachment #8353331 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1497 as attmnt 1580 at 2012-06-08 23:25:00 UTC ***

Remove offline check added in bug 954932 (bio 1499)
Attachment #8353336 - Flags: review?(florian)
Comment on attachment 8353334 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1578 at 2012-06-08 23:25:39 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353334 - Attachment is obsolete: true
Attachment #8353334 - Flags: review?(florian)
Whiteboard: [1.2-wanted]
*** Original post on bio 1497 at 2012-06-09 05:56:17 UTC ***

I could have sworn I filed this...I've known of this bug for months and we discussed it on IRC at some point...
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1497 as attmnt 1585 at 2012-06-10 20:45:00 UTC ***

Fixed line counts.
Attachment #8353342 - Flags: review?(florian)
Comment on attachment 8353336 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1580 at 2012-06-10 20:45:05 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353336 - Attachment is obsolete: true
Attachment #8353336 - Flags: review?(florian)
Whiteboard: [1.2-wanted] → [1.3-wanted]
No longer blocks: 954771
Attached patch PatchSplinter Review
*** Original post on bio 1497 as attmnt 1765 at 2012-08-01 09:51:00 UTC ***

Attachment 8353342 [details] [diff] (bio-attmnt 1585) causes Instantbird to fail to start when "Work offline" is checked in the profile manager.
The error message displayed is:
An exception occurred while initializing the core component: [Exception... "'Failure' when calling method: [imIAccountsService::processAutoLogin]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///Users/florian/buildhg/obj-instantbird-dbg/mozilla/dist/InstantbirdDebug.app/Contents/MacOS/components/imCore.js :: <TOP_LEVEL> :: line 272"  data: no]

The attached patch fixes this.
Comment on attachment 8353342 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1585 at 2012-08-01 09:51:33 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353342 - Attachment is obsolete: true
Attachment #8353342 - Flags: review?(florian)
*** Original post on bio 1497 at 2012-08-01 09:58:22 UTC ***

IRC discussion yesterday:
15:13:22 <flo> aleth: do you remember what remains to be fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=742673 ?

17:34:49 <aleth> flo: I can't reproduce that in IB (certainly no notification?), but possibly because we fixed most of it in bug 954932 (bio 1499). A complete fix would require checking in bug 954930 (bio 1497) as far as I recall.
17:34:53 <instantbot> Bug https://bugzilla.mozilla.org/show_bug.cgi?id=954932 (bio 1499) nor, --, 1.2, aletheia2, RESO FIXED, Avoid attempting to connect accounts while offline, and disconnect connecting accounts when going of
17:34:54 <instantbot> Bug https://bugzilla.mozilla.org/show_bug.cgi?id=954930 (bio 1497) nor, --, ---, aletheia2, ASSI, "-status offline" command line parameter doesn't stop accounts from connecting
17:40:21 <flo> aleth: do you remember if we identified issues with the patch waiting for review in that bug the evening we discussed it?
17:41:06 <aleth> As far as I recall the patch was r+ but you wanted the UI changes in comment #4 to go with it, which was considered part of the account manager overhaul.
17:43:33 <aleth> Looking at the patch again now I think we could probably take it anyway though, as having a different message for "offline due to necko" and "offline due to status" could be considered an enhancement, and the other two are also not really a problem

18:40:59 <aleth> Ah, maybe the UI changes were wanted so that there would still be the AUTOLOGIN_START_OFFLINE message if necko was offline?

18:42:29 <aleth> (I mean, that 1497 would not be landed until UI changes made the AUTOLOGIN_START_OFFLINE message unnecessary in that case)
*** Original post on bio 1497 at 2012-08-01 12:57:56 UTC ***

The title of this bug is not incorrect, as when started with -status offline IB still tries to connect - it's just that it fails since bug 954932 (bio 1499), so the problem is hidden:

irc: Connecting to: irc.mozilla.org:6697
irc: Disconnect
irc: onProxyAvailable called, but disconnect() was called before.

So this patch is needed for a clean solution.
Comment on attachment 8353526 [details] [diff] [review]
Patch

*** Original change on bio 1497 attmnt 1765 at 2012-08-02 17:28:08 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353526 - Flags: review+
*** Original post on bio 1497 at 2012-08-02 18:09:36 UTC ***

Pushed the patch as https://hg.instantbird.org/instantbird/rev/1b767d16b3a9 and rephrased the summary to really describe what the patch does.

Would be nice if someone could file a new UI bug and summarize the changes we want to do once we can take string changes (I think most of them are listed in comment 4 already).
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Summary: "-status offline" command line parameter doesn't stop accounts from connecting → When starting with necko offline, connect accounts as soon as the network is back. Also avoid connecting accounts when starting with "-status offline" on the command line.
Whiteboard: [1.3-wanted] → [1.2-wanted]
Target Milestone: --- → 1.2
*** Original post on bio 1497 at 2012-09-13 13:48:15 UTC ***

(In reply to comment #12)
> Would be nice if someone could file a new UI bug and summarize the changes we
> want to do once we can take string changes (I think most of them are listed in
> comment 4 already).

Listed here https://bugzilla.mozilla.org/show_bug.cgi?id=954651 (bio 1219)#c5
*** Original post on bio 1497 at 2012-10-08 12:54:08 UTC ***

The Thunderbird bug for this is https://bugzilla.mozilla.org/show_bug.cgi?id=742673
See Also: → 742673
You need to log in before you can comment on or make changes to this bug.