Closed
Bug 955164
Opened 10 years ago
Closed 10 years ago
Smiley regular expression problem in custom smiley themes
Categories
(Instantbird Graveyard :: Conversation, defect)
Instantbird Graveyard
Conversation
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: bugzilla, Assigned: aleth)
Details
Attachments
(2 files, 1 obsolete file)
215.24 KB,
image/jpeg
|
Details | |
2.17 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1734 by Steffen <steffen2705 AT web.de> at 2012-10-19 19:00:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1734 as attmnt 1995 by steffen2705 AT web.de at 2012-10-19 19:00:00 UTC *** The problem is with smileys that got a "." in them, like o.O for the actual string of the smiley the replacement works fine "o.O" -> smiley picture for anything like "oXO" where X can be anything but ".", the replacement fails somehow and just a image-not-found-dummy is shown it has something to do with the regular expression to find smileys in a conversation
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1734 at 2012-10-19 19:06:12 UTC *** The faulty regex is http://lxr.instantbird.org/instantbird/source/chat/modules/imSmileys.jsm#120 A candidate replacement is http://lxr.instantbird.org/instantbird/source/chat/modules/jsProtoHelper.jsm#511, but there may be differences due to the different use case.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1734 as attmnt 1996 at 2012-10-19 20:18:00 UTC *** So this uses the same regex for both cases. imSmileys: Add .{} to the regex and use newer $& jsProtoHelper: Remove ,- from regex as they only have special meaning inside brackets, and we escape those. Similarly # is only special for regex comments, but we already escape ?. I also removed \s (which was probably there as the nick is between \b's, but is actually not needed, unless I am missing something).
Attachment #8353755 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1734 as attmnt 1997 at 2012-10-19 20:32:00 UTC *** Sorry for forgetting about this update.
Attachment #8353756 -
Flags: review?(florian)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8353755 [details] [diff] [review] Patch *** Original change on bio 1734 attmnt 1996 at 2012-10-19 20:32:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353755 -
Attachment is obsolete: true
Attachment #8353755 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Hardware: x86 → All
Version: 1.2 → trunk
Comment 6•10 years ago
|
||
Comment on attachment 8353756 [details] [diff] [review] Patch *** Original change on bio 1734 attmnt 1997 at 2012-10-19 21:03:04 UTC *** This looks OK to me, although: - I haven't tested this. - This really feels like code that needs unit tests. Thanks for fixing this! :-)
Attachment #8353756 -
Flags: review?(florian) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 7•10 years ago
|
||
*** Original post on bio 1734 at 2012-10-20 05:26:13 UTC *** Comment on attachment 8353756 [details] [diff] [review] (bio-attmnt 1997) Patch >diff --git a/modules/imSmileys.jsm b/modules/imSmileys.jsm >- let exp = /([\][)(\\|?^$*+])/g; >+ let exp = /[[\]{}()*+?.\\^$|]/g; Doesn't this need to be \. to physically match a . to replace it with a \.?
Comment 8•10 years ago
|
||
*** Original post on bio 1734 at 2012-10-20 16:06:33 UTC *** (In reply to comment #5) > Comment on attachment 8353756 [details] [diff] [review] (bio-attmnt 1997) [details] > Patch > > >diff --git a/modules/imSmileys.jsm b/modules/imSmileys.jsm > >- let exp = /([\][)(\\|?^$*+])/g; > >+ let exp = /[[\]{}()*+?.\\^$|]/g; > Doesn't this need to be \. to physically match a . to replace it with a \.? You can check, but inside a [] I don't think we need to escape '.'.
Comment 9•10 years ago
|
||
*** Original post on bio 1734 at 2012-10-25 12:12:13 UTC *** http://hg.instantbird.org/instantbird/rev/df0392aaf7af Thanks for fixing this!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•