1009504 - Throbber persists when disconnected while joining

Status: RESOLVED FIXED

(Instantbird Graveyard :: Conversation, defect)

Description

[aleth [:aleth]]

I thought I'd taken care of this in the original patch, but I missed something.

Comment 1

[aleth [:aleth]]

Attachment: 1009504.patch

Fixes the existing code in jsProtoHelper, and modifies getDisconnected() which didn't work when disconnected during joining (because left=true is the default state for IRC).

Comment 2

[Patrick Cloke [:clokep]]

Comment on attachment 8423161 [details] [diff] [review]
1009504.patch

Review of attachment 8423161 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/modules/jsProtoHelper.jsm
@@ +577,5 @@
>  
>    _left: false,
>    get left() this._left,
>    set left(aLeft) {
> +    if (aLeft == this._left && !this.joining)

We're talking about this on IRC, but I'm halfway through my review so I'm just going to hit save: This scares me, I don't see !this.joining as a valid statement to return early. Can you please include some rationale (both in the bug and in a comment in the code) for why this is easy?

::: chat/protocols/irc/irc.js
@@ +1668,5 @@
>  
>      // Clean up each conversation: mark as left and remove participant.
>      for each (let conversation in this._conversations) {
> +      if (conversation.isChat) {
> +        conversation.joining = false;

Please add a comment above this saying something about "In case the channel was never fully joined" or something.

Comment 3

[aleth [:aleth]]

Attachment: 1009504.patch 2

Looking at this again, I think we can do without the extra safety of the logic in the left setter altogether.

Comment 4

[Patrick Cloke [:clokep]]

Comment on attachment 8423234 [details] [diff] [review]
1009504.patch 2

Review of attachment 8423234 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 5

[Patrick Cloke [:clokep]]

https://hg.mozilla.org/comm-central/rev/9f5b1f06702d