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)
Chat Core
Yahoo! Messenger
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: qheaden, Assigned: qheaden)
Details
Attachments
(2 files, 2 obsolete files)
42.53 KB,
image/png
|
Details | |
6.68 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2110 at 2013-08-15 09:44:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** 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>
Comment 2•10 years ago
|
||
*** Original post on bio 2110 at 2013-08-15 12:23:30 UTC *** What does libpurple do to these messages?
Assignee | ||
Comment 3•10 years ago
|
||
*** 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.
Assignee | ||
Comment 4•10 years ago
|
||
*** 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 | ||
Updated•10 years ago
|
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
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-
Assignee | ||
Comment 6•10 years ago
|
||
*** 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)
Assignee | ||
Comment 7•10 years ago
|
||
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 8•10 years ago
|
||
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-
Assignee | ||
Comment 9•10 years ago
|
||
*** 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)
Assignee | ||
Comment 10•10 years ago
|
||
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 11•10 years ago
|
||
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+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 12•10 years ago
|
||
*** 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.
Description
•