981884 - User icon upload fails due to removed transfer host settings

Status: RESOLVED FIXED

(Chat Core :: Yahoo! Messenger, defect)

Description

[Quentin Headen [:qheaden]]

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.

Comment 1

[Quentin Headen [:qheaden]]

Attachment: bug-981884-patch-1.patch

This patch places the transfer server information into constants for the upload code to use.

Comment 2

[Patrick Cloke [: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.

Comment 3

[Quentin Headen [:qheaden]]

Attachment: Patch 2

I've added the comment above the constants.

Comment 4

[Patrick Cloke [: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.

Comment 5

[Quentin Headen [:qheaden]]

Attachment: Patch 3

Fixed the comment from patch 2.

Comment 6

[Patrick Cloke [:clokep]]

https://hg.mozilla.org/comm-central/rev/2a377e4fb599