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: 1.2b5 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

comment:1 Changed 13 years ago by jas8522

  • Component changed from None to Message View
  • Milestone set to Needs dev review
  • Owner nobody deleted
  • Patch Status set to None
  • Version changed from 1.0 to 1.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 rageboy04

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 jas8522

  • Keywords not removed
  • Milestone changed from Needs dev review to Adium X 1.3
  • pending set to 0
  • Version changed from 1.0.2 to 1.2b5

Most of these are still an issue now.

comment:4 Changed 12 years ago by jas8522

  • Milestone changed from Adium X 1.3 to Adium X 1.3.2

comment:5 Changed 12 years ago by earthmkii

  • 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 djmori

comment:7 Changed 11 years ago by earthmkii

  • Cc stephen.holt@… removed
  • Owner set to earthmkii
  • Status changed from new to assigned

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

comment:8 Changed 11 years ago by earthmkii

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 11 years ago by evands

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 11 years ago by earthmkii

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 jas8522

  • Milestone changed from Adium 1.3.2 to Adium 1.3.3

comment:12 Changed 11 years ago by evands

  • Milestone changed from Adium 1.3.3 to Adium 1.4
  • Owner changed from earthmkii to evands
  • Status changed from assigned to new

comment:13 Changed 11 years ago by evands

  • Resolution set to fixed
  • Status changed from new to closed

(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.