Closed
Bug 955525
Opened 10 years ago
Closed 10 years ago
New contact does not get added to the correct tag
Categories
(Chat Core :: Yahoo! Messenger, defect)
Chat Core
Yahoo! Messenger
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: qheaden)
Details
Attachments
(1 file, 4 obsolete files)
3.74 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2088 at 2013-08-02 11:27:00 UTC *** STR 1) "Add contact", selecting an existing tag, say X. 2) New contact eventually shows up in a "Friends" tag which did not exist before. 3) This is the bad part: The tag X is now broken. All its existing contacts show up in the "other contacts" tag. It's impossible to select tag X from the context menu, even though it still shows up there.
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2088 at 2013-08-02 11:29:31 UTC *** After a restart, the tag X is thankfully back to working as normal and its existing contacts are still associated with it.
Reporter | ||
Comment 2•10 years ago
|
||
*** Original post on bio 2088 at 2013-08-02 13:06:10 UTC *** Also, post-restart, the contact added in 1) actually has both tag X and "Friends".
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 2088 at 2013-08-02 18:03:06 UTC *** Okay. I'm pretty sure this has to do with my TODO here http://lxr.instantbird.org/instantbird/source/chat/protocols/yahoo/yahoo-session.jsm#857. I'll work on a fix for it.
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 2088 as attmnt 2686 at 2013-08-08 18:34:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354455 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
*** Original post on bio 2088 at 2013-08-09 20:05:05 UTC *** Comment on attachment 8354455 [details] [diff] [review] (bio-attmnt 2686) Patch 1 >diff --git a/chat/protocols/yahoo/yahoo-session.jsm b/chat/protocols/yahoo/yahoo-session.jsm >--- a/chat/protocols/yahoo/yahoo-session.jsm >+++ b/chat/protocols/yahoo/yahoo-session.jsm >@@ -931,7 +931,10 @@ const YahooPacketHandler = { > packet.addValue(334, 0); > this._session.sendBinaryData(packet.toArrayBuffer()); > >- // TODO - Possibly allow different tags to be used. >+ // If someone wants to add us as a buddy, we authorize them and place >+ // them under the Friends tag. If we are adding a buddy ourselves, this >+ // method call will do nothing since the buddy will already be in our >+ // friends list. > this._account.addBuddy(Services.tags.createTag("Friends"), Why is this hard coded to friends? Doesn't the add buddy dialogue (Why is this a dialogue again? :P) let you specify the tag to add it to? >diff --git a/chat/protocols/yahoo/yahoo.js b/chat/protocols/yahoo/yahoo.js >--- a/chat/protocols/yahoo/yahoo.js >+++ b/chat/protocols/yahoo/yahoo.js >@@ -281,6 +281,9 @@ YahooAccount.prototype = { > > // Called when the user adds a new contact within Instantbird. > addBuddy: function(aTag, aName) { >+ // Don't add duplicate buddies. >+ if (this._buddies.has(aName)) >+ return; When do we expect this to happen? It should be in the comment. (I.e. is this an error or an expected condition?)
Comment 6•10 years ago
|
||
Comment on attachment 8354455 [details] [diff] [review] Patch 1 *** Original change on bio 2088 attmnt 2686 at 2013-08-11 18:41:53 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354455 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2088 as attmnt 2693 at 2013-08-12 11:25:00 UTC *** This patch localizes the group tag "Friends", but it still uses the default tag because it is for buddy authorization requests (when a buddy wants to be your friend). Right now, the buddy auth UI doesn't allow specifying of tags.
Attachment #8354462 -
Flags: review?(clokep)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8354455 [details] [diff] [review] Patch 1 *** Original change on bio 2088 attmnt 2686 at 2013-08-12 11:25:06 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354455 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
Comment on attachment 8354462 [details] [diff] [review] Patch 2 *** Original change on bio 2088 attmnt 2693 at 2013-08-12 12:31:42 UTC *** >- // Called when the user adds a new contact within Instantbird. >+ // Called when the user adds or authorizes a new contact within Instantbird. I should have caught this during initial review, but since we're not touching this comment: it shouldn't refere to "Instantbird" since it's in chat/ and is used by Thunderbird as well. You can probably just remove the "within Instantbird" part.
Attachment #8354462 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 2088 as attmnt 2694 at 2013-08-12 12:39:00 UTC *** Fixed.
Attachment #8354463 -
Flags: review?(clokep)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8354462 [details] [diff] [review] Patch 2 *** Original change on bio 2088 attmnt 2693 at 2013-08-12 12:39:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354462 -
Attachment is obsolete: true
Comment 12•10 years ago
|
||
Comment on attachment 8354463 [details] [diff] [review] Patch 3 *** Original change on bio 2088 attmnt 2694 at 2013-08-12 13:27:06 UTC *** >diff --git a/chat/protocols/yahoo/yahoo-session.jsm b/chat/protocols/yahoo/yahoo-session.jsm >- // TODO - Possibly allow different tags to be used. >- this._account.addBuddy(Services.tags.createTag("Friends"), >- this.userName); >+ // If someone wants to add us as a buddy, place them under the default >+ // "Friends" tag. If we are adding someone as a buddy, this method will >+ // do nothing since the buddy will already be in our list. >+ let tag = Services.tags.createTag(_("buddy.auth.defaultGroup")); >+ this._account.addBuddy(tag, this.userName); We discussed on IRC how the tag creation should be moved to addBuddy.
Attachment #8354463 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 13•10 years ago
|
||
*** Original post on bio 2088 as attmnt 2695 at 2013-08-12 17:33:00 UTC ***
> >diff --git a/chat/protocols/yahoo/yahoo-session.jsm b/chat/protocols/yahoo/yahoo-session.jsm
> >- // TODO - Possibly allow different tags to be used.
> >- this._account.addBuddy(Services.tags.createTag("Friends"),
> >- this.userName);
> >+ // If someone wants to add us as a buddy, place them under the default
> >+ // "Friends" tag. If we are adding someone as a buddy, this method will
> >+ // do nothing since the buddy will already be in our list.
> >+ let tag = Services.tags.createTag(_("buddy.auth.defaultGroup"));
> >+ this._account.addBuddy(tag, this.userName);
> We discussed on IRC how the tag creation should be moved to addBuddy.
We now see that addBuddy is an interface method, and as a result, it expects an already created tag to be passed as the first parameter. So I moved the duplication check outside of addBuddy using the new hasBuddy method, and if the buddy already exists, it prevents creation of the tag.
Attachment #8354464 -
Flags: review?(clokep)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8354463 [details] [diff] [review] Patch 3 *** Original change on bio 2088 attmnt 2694 at 2013-08-12 17:33:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354463 -
Attachment is obsolete: true
Comment 15•10 years ago
|
||
Comment on attachment 8354464 [details] [diff] [review] Patch 4 *** Original change on bio 2088 attmnt 2695 at 2013-08-12 18:52:38 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354464 -
Flags: review?(clokep) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 16•10 years ago
|
||
*** Original post on bio 2088 as attmnt 2705 at 2013-08-13 00:40:00 UTC *** Small update from v4 of the patch that changes the default group from "Friends" to "Contacts".
Attachment #8354474 -
Flags: review?(clokep)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8354464 [details] [diff] [review] Patch 4 *** Original change on bio 2088 attmnt 2695 at 2013-08-13 00:40:55 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354464 -
Attachment is obsolete: true
Comment 18•10 years ago
|
||
Comment on attachment 8354474 [details] [diff] [review] Patch 5 *** Original change on bio 2088 attmnt 2705 at 2013-08-13 00:46:49 UTC *** OK, this looks good. Let's file a follow up to not have a Contacts tag in every protocol.
Attachment #8354474 -
Flags: review?(clokep) → review+
Comment 19•10 years ago
|
||
*** Original post on bio 2088 at 2013-08-14 13:49:15 UTC *** http://hg.instantbird.org/instantbird/rev/1eea25566706 (In reply to comment #12) > Let's file a follow up to not have a Contacts tag in every protocol. This is bug 955543 (bio 2105).
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•