Closed Bug 976177 Opened 6 years ago Closed 5 years ago

JS-Yahoo doesn't implement icon removal

Categories

(Chat Core :: Yahoo! Messenger, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mayanktg, Assigned: qheaden)

References

Details

Attachments

(1 file, 1 obsolete file)

When a user wants to remove his user icon, Yahoo IM is unable to do that.
This is blocking the current user icon removal Bug 954216.
The code for this lies here: http://mxr.mozilla.org/comm-central/source/chat/protocols/yahoo/yahoo-session.jsm#312
Blocks: 954216
(In reply to Patrick Cloke [:clokep] from bug #954216, comment #15)
> (In reply to Mayank Kumar [:mayanktg] from bug #954216, comment #14)
> > In the code at 
> > http://mxr.mozilla.org/comm-central/source/chat/protocols/yahoo/yahoo-
> > session.jsm#312
> > I'm adding a remove user icon feature I want to know whether this supports
> > user icon removal in any way?
> 
> For reference:
> http://lxr.instantbird.org/instantbird/source/purple/libpurple/protocols/
> yahoo/yahoo_picture.c#542
Flags: needinfo?(qheaden)
Attached patch Patch 1 (obsolete) — Splinter Review
This code seems to do the trick in removing the icon from your profile in JS-Yahoo. I logged in on another machine under another account, and using the official Yahoo! Messenger client, I was able to see my icon go away after removing it using JS-Yahoo.
Assignee: nobody → qheaden
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8388841 - Flags: review?(clokep)
Flags: needinfo?(qheaden)
Comment on attachment 8388841 [details] [diff] [review]
Patch 1

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

I have a couple of questions, so not r+ or r-ing this yet.

::: chat/protocols/yahoo/yahoo-session.jsm
@@ +310,5 @@
>    },
>  
>    setProfileIcon: function(aFileName) {
> +    // If we have an empty filename, remove the icon from the server.
> +    if (aFileName == null || aFileName == "") {

Are we expecting an empty filename? Do other protocols check for this? Should we just use !aFileName?

@@ +313,5 @@
> +    // If we have an empty filename, remove the icon from the server.
> +    if (aFileName == null || aFileName == "") {
> +      let packet = new YahooPacket(kPacketType.AvatarUpdate, 0, this.sessionId);
> +      packet.addValue(3, this._account.cleanUsername);
> +      packet.addValue(213, 0);

I assume we have no idea what these mean and it's just copied from a wireshark thing?
Attachment #8388841 - Flags: feedback+
Comment on attachment 8388841 [details] [diff] [review]
Patch 1

Please re-request review if you disagree with my comments.
Attachment #8388841 - Flags: review?(clokep)
Blocks: 955574
Attached patch Patch 2Splinter Review
This patch addresses the previous comments. As for the packet key-value numbers, no, we don't know what they mean. They are just the keys I found while using Wireshark.
Attachment #8388841 - Attachment is obsolete: true
Attachment #8447200 - Flags: review?(clokep)
Attachment #8447200 - Flags: review?(clokep) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/144a20da5ab5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.