Closed
Bug 1037379
Opened 10 years ago
Closed 10 years ago
Newly added contact shown as status unknown after reconnecting
Categories
(Chat Core :: IRC, defect)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.6
People
(Reporter: florian, Assigned: aleth)
Details
Attachments
(1 file, 1 obsolete file)
3.49 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Keywords: regression
Comment 3•10 years ago
|
||
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. :(
Assignee | ||
Comment 4•10 years ago
|
||
(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•10 years ago
|
||
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.
Attachment #8454530 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 6•10 years ago
|
||
Comments updated.
Attachment #8454530 -
Attachment is obsolete: true
Attachment #8454587 -
Flags: review?(clokep)
Comment 7•10 years ago
|
||
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.
Attachment #8454587 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/4e595a18812b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in
before you can comment on or make changes to this bug.
Description
•