Adium

Opened 13 years ago

Closed 11 years ago

#6440 closed defect (fixed)

Emoticons do not display after some special characters

Reported by: roowie Owned by: evands
Milestone: Adium 1.4 Component: Message View
Version: Severity: normal
Keywords: emoticons display special character Cc:
Patch Status:

Description

I detected that sometimes some emoticons won't display when having other emoticons or characters in front.

Example of characters that will destroy the last emoticon: #, @, %, & and / Example of emoticions that will destroy the second from the last: (#), (@), (%), (&) and (/)

Attachments (1)

adium-emoticons-bug.png (69.3 KB) - added by roowie 13 years ago.

Download all attachments as: .zip

Change History (14)

Changed 13 years ago by roowie

Attachment: adium-emoticons-bug.png added

comment:1 Changed 13 years ago by Jordan

Component: NoneMessage View
Milestone: Needs dev review
Owner: nobody deleted
Patch Status: None
Version: 1.01.0.2

I can confirm this. With the @, perhaps it's looking for an email address or something along those lines? It's easiest to reproduce if you look at his picture and try to mimic that. Very odd.

comment:2 Changed 12 years ago by Ankit Singla

I've also found that before certain characters, emoticons display incorrectly. For instance, ":P*" shows up right, but ".+:P" does not, where .+ represents any number of (>1) characters. One weird thing I found is that ":P:P*" shows up correctly (two tongue faces" but ":):P*" does not and ":):)*" does not. Both of the second show up as a smile and then ":P*" (no smiley).

(wikiformatting applied already)

comment:3 Changed 12 years ago by Jordan

Keywords: not removed
Milestone: Needs dev reviewAdium X 1.3
pending: 0
Version: 1.0.21.2b5

Most of these are still an issue now.

comment:4 Changed 12 years ago by Jordan

Milestone: Adium X 1.3Adium X 1.3.2

comment:5 Changed 12 years ago by Stephen Holt

Cc: stephen.holt@… added

I'm adding myself as a cc: on this ticket. Did some work on the emoticon text replacement back in the day, but need to re-familiarize myself with it before I take an assignment.

comment:6 Changed 12 years ago by Carlos Morales

comment:7 Changed 12 years ago by Stephen Holt

Cc: stephen.holt@… removed
Owner: set to Stephen Holt
Status: newassigned

I'll take this one, if there are no objections.

comment:8 Changed 12 years ago by Stephen Holt

When a member of +[NSCharacterSet punctuationCharacterSet] begins a string, the recursive parsing algorithm trips up and exits early.

Those characters are:

! " % ' ( ) , - . / : ; ? [ \ ] { }

(via http://www.cocoadev.com/index.pl?NSCharacterSet)

It just so happens that those characters are often used in emoticons. So, when we use NSScanner to eat up punc, it's sucking in parts of emoticons too.

comment:9 Changed 12 years ago by Evan Schoenberg

Ouch. Perhaps we should skim over whitespace but leave punctuation intact; it means a few more loops through the process for a small minority of strings, but I would think that would let emoticonification occur properly.

comment:10 Changed 12 years ago by Stephen Holt

amending that comment a bit, it's not exactly punctuationCharacterSet: that's doing it, but characters in emoticonStartCharacterSet.

Inverting that set and forming an intersection with the punc and whitespace sets solves the issue - but I think it effectively just nullifies the punc. set.

comment:11 Changed 11 years ago by Jordan

Milestone: Adium 1.3.2Adium 1.3.3

comment:12 Changed 11 years ago by Evan Schoenberg

Milestone: Adium 1.3.3Adium 1.4
Owner: changed from Stephen Holt to Evan Schoenberg
Status: assignednew

comment:13 Changed 11 years ago by Evan Schoenberg

Resolution: fixed
Status: newclosed

(In [25792]) Fix a longstanding bug in the emoticon controller which mishandled look-ahead emoticon parsing, and simplified the code in the process. There was some bizarre recursive pass-by-reference occurring, causing among other things the 'currentLocation' in the string to actually end up way beyond the end of the string by the time the recursivity finished.

I'm not going to merge to adium-1.3 because I'd like further stress testing to make sure I didn't introduce some new bug while fixing some edge cases.

Fixes #6440

Note: See TracTickets for help on using tickets.