Closed Bug 955548 Opened 10 years ago Closed 10 years ago

Formatted Incoming Messages Always Shown With Large Letters

Categories

(Chat Core :: Yahoo! Messenger, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: qheaden, Assigned: qheaden)

Details

Attachments

(2 files, 2 obsolete files)

*** Original post on bio 2110 at 2013-08-15 09:44:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached image Screenshot of Issue
*** Original post on bio 2110 as attmnt 2720 at 2013-08-15 09:44:00 UTC ***

Whenever you chat with someone who uses a client that sends HTML formatted messages, the message is displayed in the window using extremely large letters. Also, the second account I was chatting with was sending messages with the Yahoo! Messenger official web client (located within Yahoo! Mail). That client sends formatted messages.

Here is an example formatted message: <font face="Arial" size="10">Test</font>
*** Original post on bio 2110 at 2013-08-15 12:23:30 UTC ***

What does libpurple do to these messages?
*** Original post on bio 2110 at 2013-08-16 16:21:47 UTC ***

It seems that Yahoo's formatted messages have a weird size value. Normally, HTML font tags accept a size value between 1 and 7 (http://www.w3schools.com/tags/att_font_size.asp). Yahoo's font tags contain the real size of the text. For example, instead of sending size="3" for normal sized text, it will send size="10" (the actual pt size), which is over 7, making the text very large.

This is why the libpurple Yahoo code contains these defines: http://lxr.instantbird.org/instantbird/source/purple/libpurple/protocols/yahoo/util.c#686.
Attached patch Patch 1 (obsolete) — Splinter Review
*** Original post on bio 2110 as attmnt 2739 at 2013-08-20 10:23:00 UTC ***

This patch uses JS regexp in order to find font tags, and correct the given sizes, putting them in a HTML-compliant form. If tests are desired, I can add them in an additional patch.
Attachment #8354508 - Flags: review?(clokep)
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment on attachment 8354508 [details] [diff] [review]
Patch 1

*** Original change on bio 2110 attmnt 2739 at 2013-08-20 10:37:39 UTC ***

Please include tests, I'm not convinced this will work on a message such as: <font size="x">size="7"

Possible fixes:
- More complex regexp: <font[^>]+size="\d"
- Better searching in _fixFontSize for the last <font tag

Also, ensure this works with multiple font tags if that's possible?
Attachment #8354508 - Flags: review?(clokep) → review-
Attached patch Patch 2 (obsolete) — Splinter Review
*** Original post on bio 2110 as attmnt 2740 at 2013-08-20 11:38:00 UTC ***

This patch uses better regex to prevent messages containing size="<digit>" from getting changed. It also handles multiple font tags. A test was added to ensure that the _fixFontSize() method works as expected.
Attachment #8354509 - Flags: review?(clokep)
Comment on attachment 8354508 [details] [diff] [review]
Patch 1

*** Original change on bio 2110 attmnt 2739 at 2013-08-20 11:38:34 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354508 - Attachment is obsolete: true
Comment on attachment 8354509 [details] [diff] [review]
Patch 2

*** Original change on bio 2110 attmnt 2740 at 2013-08-20 12:14:02 UTC ***

As Florian said on IRC:
"note: the test in the patch doesn't check that things still work if there's no face attribute, or if size isn't the last attribute, or if there's a font tag without size attribute."

These are all important and should be tested.
Attachment #8354509 - Flags: review?(clokep) → review-
Attached patch Patch 3Splinter Review
*** Original post on bio 2110 as attmnt 2743 at 2013-08-20 16:27:00 UTC ***

This patch reworks the test code so that it covers more cases.
Attachment #8354512 - Flags: review?(clokep)
Comment on attachment 8354509 [details] [diff] [review]
Patch 2

*** Original change on bio 2110 attmnt 2740 at 2013-08-20 16:27:10 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354509 - Attachment is obsolete: true
Comment on attachment 8354512 [details] [diff] [review]
Patch 3

*** Original change on bio 2110 attmnt 2743 at 2013-08-20 16:36:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354512 - Flags: review?(clokep) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 2110 at 2013-08-21 01:48:06 UTC ***

Thanks for fixing this!

http://hg.instantbird.org/instantbird/rev/488b2bfed608
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.