Closed Bug 981884 Opened 6 years ago Closed 6 years ago

User icon upload fails due to removed transfer host settings

Categories

(Chat Core :: Yahoo! Messenger, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: qheaden, Assigned: qheaden)

Details

Attachments

(1 file, 2 obsolete files)

In a past patch, the xfer_host and xfer_port settings for the JS-Yahoo plugin were commented out. It was forgotten that these settings are used by the icon upload code to upload a user icon to the Yahoo servers. It needs to be added back, but not as a setting. Rather, it can be hard coded into the plug-in.
Attached patch bug-981884-patch-1.patch (obsolete) — Splinter Review
This patch places the transfer server information into constants for the upload code to use.
Attachment #8388837 - Flags: review?(clokep)
Comment on attachment 8388837 [details] [diff] [review]
bug-981884-patch-1.patch

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

This was bad on my part. :( A trivial thing to fix and it'll be an r+. Thanks for looking at this!

::: chat/protocols/yahoo/yahoo-session.jsm
@@ +29,4 @@
>  const kPacketHeaderSize = 20;
>  const kProfileIconWidth = 96;
>  
> +const kFileTransferHost = "filetransfer.msg.yahoo.com";

Please include the comment you had about getting the preferences, I think it's valuable to understand why we're defining these constants.
Attachment #8388837 - Flags: review?(clokep) → review-
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Attached patch Patch 2 (obsolete) — Splinter Review
I've added the comment above the constants.
Attachment #8388837 - Attachment is obsolete: true
Attachment #8389665 - Flags: review?(clokep)
Comment on attachment 8389665 [details] [diff] [review]
Patch 2

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

::: chat/protocols/yahoo/yahoo-session.jsm
@@ +29,5 @@
>  const kPacketHeaderSize = 20;
>  const kProfileIconWidth = 96;
>  
> +// We use constants to store the file transfer server information instead of
> +// preferences since these values aren't to be changed by the user.

The comment I meant to include was "The Yahoo! file transfer server is used to upload the user icon." Sorry for being unclear.
Attachment #8389665 - Flags: review?(clokep) → review-
Attached patch Patch 3Splinter Review
Fixed the comment from patch 2.
Attachment #8389665 - Attachment is obsolete: true
Attachment #8418118 - Flags: review?(clokep)
Attachment #8418118 - Flags: review?(clokep) → review+
https://hg.mozilla.org/comm-central/rev/2a377e4fb599
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.