Closed Bug 954771 Opened 10 years ago Closed 10 years ago

Connect button is disabled after an account spends more than half a second in the disconnecting state

Categories

(Instantbird Graveyard :: Account manager, defect)

x86
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

Details

(Whiteboard: [1.2-blocking])

Attachments

(2 files, 1 obsolete file)

*** Original post on bio 1338 at 2012-03-15 14:25:00 UTC ***

Very visible with JS-IRC...
Whiteboard: [1.2-blocking]
Depends on: 954930
No longer depends on: 954930
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1338 as attmnt 1692 at 2012-06-25 15:27:00 UTC ***

Can't see a prettier way of doing this...
Attachment #8353449 - Flags: review?(clokep)
*** Original post on bio 1338 at 2012-06-25 15:32:33 UTC ***

I remember suggesting an easier solution on IRC: just always disable/reenable both buttons at once. Would that work?
Comment on attachment 8353449 [details] [diff] [review]
Patch

*** Original change on bio 1338 attmnt 1692 at 2012-06-25 15:33:26 UTC ***

I really don't know this code.
Attachment #8353449 - Flags: review?(clokep) → review?(florian)
*** Original post on bio 1338 at 2012-06-25 15:35:49 UTC ***

(In reply to comment #2)
> I remember suggesting an easier solution on IRC: just always disable/reenable
> both buttons at once. Would that work?

I'm not sure I understand.

The problem as I saw it is that when connecting, we want the Disconnect button to be enabled only after a brief delay, to avoid double-click errors (but no later, so we can abort connecting). When disconnecting, we want the Connect button to be enabled only after a delay AND when the account has actually disconnected.
Comment on attachment 8353449 [details] [diff] [review]
Patch

*** Original change on bio 1338 attmnt 1692 at 2012-08-03 13:37:54 UTC ***

I don't remember exactly what the idea I had to handle this was, but this patch has more setTimeout calls than reasonable. Especially the _buttonTimer callback calling itself again with a timeout seems wrong.
Attachment #8353449 - Flags: review?(florian) → review-
Attached patch PatchSplinter Review
*** Original post on bio 1338 as attmnt 1780 at 2012-08-03 16:08:00 UTC ***

Patch following my suggestion of comment 2.
Attachment #8353541 - Flags: review?(bugzilla)
Comment on attachment 8353449 [details] [diff] [review]
Patch

*** Original change on bio 1338 attmnt 1692 at 2012-08-03 16:08:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353449 - Attachment is obsolete: true
Assignee: nobody → florian
Comment on attachment 8353541 [details] [diff] [review]
Patch

*** Original change on bio 1338 attmnt 1780 at 2012-08-03 17:09:28 UTC ***

Simplifies the previously convoluted code and works well :)
Attachment #8353541 - Flags: review?(bugzilla) → review+
*** Original post on bio 1338 as attmnt 1788 at 2012-08-04 22:43:00 UTC ***

Mike reviewed the same patch for Thunderbird and suggested some further simplification of the code: https://bugzilla.mozilla.org/show_bug.cgi?id=780173#c1
Attachment #8353547 - Flags: review?(bugzilla)
*** Original post on bio 1338 at 2012-08-04 22:44:08 UTC ***

The fix as landed as https://hg.instantbird.org/instantbird/rev/871e12a7e1cf and I don't think this needs to stay open for the follow-up, so resolving as FIXED.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
Comment on attachment 8353547 [details] [diff] [review]
Follow-up addressing mconley's suggestion

*** Original change on bio 1338 attmnt 1788 at 2012-08-05 20:08:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353547 - Flags: review?(bugzilla) → review+
*** Original post on bio 1338 at 2012-08-21 13:46:21 UTC ***

Follow-up checked-in as https://hg.instantbird.org/instantbird/rev/fecf533aba2c (after the 1.2 release, so it will be in 1.3).
You need to log in before you can comment on or make changes to this bug.