Closed Bug 953629 Opened 10 years ago Closed 10 years ago

disable buttons in account manager when offline

Categories

(Instantbird Graveyard :: Account manager, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: romain)

References

Details

Attachments

(2 files, 4 obsolete files)

*** Original post on bio 182 at 2009-04-24 20:54:00 UTC ***

When the application is in offline mode (detected automatically based on the network state, or set manually with a command line switch), it's not possible to connect accounts. Click on the connect button in the account manager throws an exception.
We should disable the button in this case, and probably add a message somewhere explaining why it's not possible to connect accounts now.
Blocks: 953623
*** Original post on bio 182 at 2009-04-25 13:32:35 UTC ***

A notification bar like on the add-on manager ("Restart Instantbird to...")could be a good idea.

That is
-> A message saying that you're currently disconnected.
-> A button to try to "go online". Well, I admit it doesn't help against disconnected network cables;) but maybe against "offline by command line parameter" or "dial-up to connect to the internet".
-> Possibility to click it away when you've noticed it.
*** Original post on bio 182 at 2009-04-25 14:04:05 UTC ***

(In reply to comment #1)

We already have a notification bar for the case of "offline because of the command line switch", with a button "Connect Now".
If you are offline because the network is disconnected, I don't really see how a button can help here...
*** Original post on bio 182 at 2009-08-02 22:28:34 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
Attached patch Patch V1 (obsolete) — Splinter Review
*** Original post on bio 182 as attmnt 167 at 2009-08-05 12:07:00 UTC ***

Added two methods on account.xml, disableConnectButton() and restoreConnectButton().
This is visual only, handling disconnections and set account as disconnected when the network is planed on bug 953573 (bio 127) (suspend to RAM).

A little issue I discovered is that when you click the "Connect Now" message, then the focus is somewhere where you cannot use up and down arrows, if it is considered as an issue I can handle to have a patch for this too.
Attachment #8351911 - Flags: review?(florian)
Assignee: nobody → romain
Status: NEW → ASSIGNED
Comment on attachment 8351911 [details] [diff] [review]
Patch V1

*** Original change on bio 182 attmnt 167 at 2009-08-05 21:27:56 UTC ***

A bit too much code duplication for me :-(.

What about adding an offline property to the account binding?

When setting offline to true, the setter would disable the button and set an "offline" attribute (which may be useful to some theme authors). When setting to false, it would just remove the offline attribute and set the disabled-ness of the connect button according to the missingness of the protocol plugin.

In accounts.js, you can add a setOffline method that would take a boolean as parameter.
You will call it from your observer, and when loading the window.

By the way, the case of a new account created after the window was opened may be buggy both with your current patch and with my proposed changes.
Attachment #8351911 - Flags: review?(florian) → review-
Attached patch Patch V2 (obsolete) — Splinter Review
*** Original post on bio 182 as attmnt 171 at 2009-08-06 01:11:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351915 - Flags: review?(florian)
Comment on attachment 8351911 [details] [diff] [review]
Patch V1

*** Original change on bio 182 attmnt 167 at 2009-08-06 01:11:40 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351911 - Attachment is obsolete: true
Attached patch Patch V3 (obsolete) — Splinter Review
*** Original post on bio 182 as attmnt 172 at 2009-08-06 08:52:00 UTC ***

As discussed on IM, some few coding style changes.
Attachment #8351916 - Flags: review?(florian)
Comment on attachment 8351915 [details] [diff] [review]
Patch V2

*** Original change on bio 182 attmnt 171 at 2009-08-06 08:52:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351915 - Attachment is obsolete: true
Attachment #8351915 - Flags: review?(florian)
Attached patch Patch V4 (obsolete) — Splinter Review
*** Original post on bio 182 as attmnt 173 at 2009-08-06 13:24:00 UTC ***

The previous patch does not work as expected...
Attachment #8351917 - Flags: review?(florian)
Comment on attachment 8351916 [details] [diff] [review]
Patch V3

*** Original change on bio 182 attmnt 172 at 2009-08-06 13:24:40 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351916 - Attachment is obsolete: true
Attachment #8351916 - Flags: review?(florian)
Comment on attachment 8351917 [details] [diff] [review]
Patch V4

*** Original change on bio 182 attmnt 173 at 2009-08-06 13:42:13 UTC ***

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

>+     <property name="offline">

>+       <setter>
>+         <![CDATA[
>+           let button =
>+             document.getAnonymousElementByAttribute(this, "anonid", "connect");
Move this just before the lines where it's used.

>+           let disabled = ;
Isn't that line you forgot to remove causing a syntax error?

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

>+    else if (aTopic == "network:offline-status-changed") {
>+      let isOffline = aData == "offline";
>+      this.setOffline(isOffline);
You don't need the isOffline variable.

r=me with that fixed!
Attachment #8351917 - Flags: review?(florian) → review+
Attached patch Patch V5Splinter Review
*** Original post on bio 182 as attmnt 174 at 2009-08-06 13:48:00 UTC ***

I forgot to make a new "hg diff" after removing the "let disabled = ;".
I also applied the changed described in the previous comment.
And I also tested that it is still working and that the hg diff is correct :).
Attachment #8351918 - Flags: review+
Comment on attachment 8351917 [details] [diff] [review]
Patch V4

*** Original change on bio 182 attmnt 173 at 2009-08-06 13:48:08 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351917 - Attachment is obsolete: true
*** Original post on bio 182 at 2009-08-07 01:39:57 UTC ***

https://hg.instantbird.org/instantbird/rev/0fb3f4668878
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: 0.2 → 0.2b1
Attached patch Follow upSplinter Review
*** Original post on bio 182 as attmnt 192 at 2009-08-14 01:03:00 UTC ***

Fix an issue: when autologin is enabled and no account has crashed, the connect button of an unknown protocol was not disabled.

And add a semicolon after var connectButton = { [...] }
Attachment #8351936 - Flags: review?(florian)
Comment on attachment 8351936 [details] [diff] [review]
Follow up

*** Original change on bio 182 attmnt 192 at 2009-08-14 01:08:46 UTC ***

Thanks for taking care of this!
Attachment #8351936 - Flags: review?(florian) → review+
*** Original post on bio 182 at 2009-08-14 01:19:38 UTC ***

Comment on attachment 8351936 [details] [diff] [review] (bio-attmnt 192)
Follow up

https://hg.instantbird.org/instantbird/rev/d6d3e8f21513
You need to log in before you can comment on or make changes to this bug.