Last Comment Bug 767155 - JS error when the contact of an ongoing conversation signs off
: JS error when the contact of an ongoing conversation signs off
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 17.0
Assigned To: Florian Quèze [:florian] [:flo]
Depends on:
  Show dependency treegraph
Reported: 2012-06-21 14:26 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-07-23 13:02 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (2.64 KB, patch)
2012-07-23 05:17 PDT, Florian Quèze [:florian] [:flo]
mconley: review+
bwinton: approval‑comm‑aurora+
bwinton: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2012-06-21 14:26:59 PDT
[Exception... "'Removing a contact that isn't here?' when calling method: [nsIObserver::observe]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: file:///Users/florian/buildhg/comm-central/obj-tbird/mozilla/dist/ :: <TOP_LEVEL> :: line 857"  data: no]

When an account signs off, we try to remove all of its contacts from the "online contacts" group to add them in the "offline contacts" group. However, it's possible that the contact wasn't listed in the "online contacts" group if there was an ongoing conversation with that contact.
Comment 1 Florian Quèze [:florian] [:flo] 2012-07-23 05:17:02 PDT
Created attachment 644899 [details] [diff] [review]

This actually has a visible effect in addition to the error in the console:
when a contact of an ongoing conversation signs-off, the contact is added in the "Offline Contacts" group and then when the contact signs back on, it appears in the "Online Contacts" group; so we have duplicated contacts in the list.
Comment 2 Florian Quèze [:florian] [:flo] 2012-07-23 05:46:32 PDT
Note: The patch in bug 775105 will bitrot this patch by removing this.onlineContacts from chat-messenger-overlay.js, but it shouldn't be a problem for the review.
Comment 3 Mike Conley (:mconley) 2012-07-23 07:21:01 PDT
Comment on attachment 644899 [details] [diff] [review]

Review of attachment 644899 [details] [diff] [review]:

I don't currently have the capability to test this patch manually, but the code looks good, and I assume you tested it. :)

r=me by inspection.
Comment 4 Blake Winton (:bwinton) (:☕️) 2012-07-23 11:53:08 PDT
Comment on attachment 644899 [details] [diff] [review]

I don't think we need to take this for beta, but having it will be a little cleaner.
Comment 6 Mike Conley (:mconley) 2012-07-23 13:02:30 PDT
Backed out of comm-beta since we landed on a SeaMonkey relbranch (oops).

Re-landed on comm-beta as:

Note You need to log in before you can comment on or make changes to this bug.