1037379 - Newly added contact shown as status unknown after reconnecting

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[Florian Quèze [:florian]]

Assumed steps to reproduce:
1. Add an irc nick to your contact list. See that the presence info is shown.
2. Disconnect the account.
3. Reconnect the account.

Expected result:
presence still shown after 3.

Actual result:
contact shown as status unknown.

Comment 1

[aleth [:aleth]]

This happens because trackBuddyWatch doesn't add the nick to the trackqueue on the account, which is used to initialize the watch list after a reconnect.

Comment 2

[aleth [:aleth]]

Attachment: trackbuddy.diff

Comment 3

[Patrick Cloke [:clokep]]

Comment on attachment 8454530 [details] [diff] [review]
trackbuddy.diff

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

::: chat/protocols/irc/ircWatchMonitor.jsm
@@ +41,5 @@
>    return true;
>  }
>  
>  function trackBuddyWatch(aNicks) {
> +  if (!Array.isArray(aNicks)) {

I dislike how we're essentially abusing an array to tell us whether it's the initial call or not.

@@ +44,5 @@
>  function trackBuddyWatch(aNicks) {
> +  if (!Array.isArray(aNicks)) {
> +    // We update the trackqueue if an individual nick is being added,
> +    // so the nick will also be monitored after a reconnect.
> +    Object.getPrototypeOf(this).trackBuddy.call(this, aNicks);

Normally don't we hard-code the full object here? I assume you can't do this because it's in a different file (and if you included it you'd get circular references). Is this correct?

@@ +286,5 @@
>      }
>    }
>  };
>  
>  function trackBuddyMonitor(aNicks) {

Not your fault, but lots of code duplication. :(

Comment 4

[aleth [:aleth]]

(In reply to Patrick Cloke [:clokep] from comment #3)
> I dislike how we're essentially abusing an array to tell us whether it's the
> initial call or not.
...
> Not your fault, but lots of code duplication. :(

I agree, but this bugfix isn't the place to clean up this file ;)

> > +    Object.getPrototypeOf(this).trackBuddy.call(this, aNicks);
> 
> Normally don't we hard-code the full object here? I assume you can't do this
> because it's in a different file (and if you included it you'd get circular
> references). Is this correct?

Yes exactly.

Comment 5

[Patrick Cloke [:clokep]]

Comment on attachment 8454530 [details] [diff] [review]
trackbuddy.diff

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

::: chat/protocols/irc/ircWatchMonitor.jsm
@@ +41,5 @@
>    return true;
>  }
>  
>  function trackBuddyWatch(aNicks) {
> +  if (!Array.isArray(aNicks)) {

Please put a comment here saying that this avoids the initial call or something like that.

@@ +42,5 @@
>  }
>  
>  function trackBuddyWatch(aNicks) {
> +  if (!Array.isArray(aNicks)) {
> +    // We update the trackqueue if an individual nick is being added,

Nit: trackQueue.

Comment 6

[aleth [:aleth]]

Attachment: trackbuddy.diff 2

Comments updated.

Comment 7

[Patrick Cloke [:clokep]]

Comment on attachment 8454587 [details] [diff] [review]
trackbuddy.diff 2

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

Thanks for figuring this out!

::: chat/protocols/irc/ircWatchMonitor.jsm
@@ +41,5 @@
>    return true;
>  }
>  
>  function trackBuddyWatch(aNicks) {
> +  // An array (the whole trackQueue) is passed only on connection.

Before pushing can you please change this to "aNicks is an array when (WATCH or MONITOR) is initialized, but then is a single nick afterward." or something more explicit.

Comment 8

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/4e595a18812b