User icon upload fails due to removed transfer host settings

RESOLVED FIXED in 1.6

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: qheaden, Assigned: qheaden)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Posted 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
(Assignee)

Comment 3

5 years ago
Posted 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-
(Assignee)

Comment 5

5 years ago
Posted 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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.