[wp-trac] [WordPress Trac] #20124: Smilies Fail Combinations

[WordPress Trac]

#20124: Smilies Fail Combinations
-----------------------------------------+------------------------------
 Reporter:  soulseekah                   |       Owner:
     Type:  defect (bug)                 |      Status:  new
 Priority:  normal                       |   Milestone:  Awaiting Review
Component:  Formatting                   |     Version:  3.3.1
 Severity:  normal                       |  Resolution:
 Keywords:  smilies, regexp, formatting  |
-----------------------------------------+------------------------------

Comment (by soulseekah):

 This is the regular expression that is matched against the content:

 {{{
 /(?:\s|^);(?:\-\)|\))|(?:\s|^)\:(?:\||x|wink\:|twisted\:|smile\:|shock\:|sad\:|roll\:|razz\:|oops\:|o|neutral\:|mrgreen\:|mad\:|lol\:|idea\:|grin\:|evil\:|eek\:|cry\:|cool\:|arrow\:|P|D|\?\?\?\:|\?\:|\?|\-\||\-x|\-o|\-P|\-D|\-\?|\-\)|\-\(|\)|\(|\!\:)|(?:\s|^)8(?:O|\-O|\-\)|\))(?:\s|$)/m
 }}}

 This can be simplified visually to

 {{{
 (?:\s|^);(...variants...)
 or
 (?:\s|^):(...variants...)
 or
 (?:\s|^)8(...variants...)(?:\s|$)
 }}}


 The problem has been pinpointed to the last {{{ (?:\s|$) }}} clause. This
 is attached to "8"-type smilies and consumes the single trailing
 whitespace following it, meaning that anything that comes afterwards
 matching any of the groups, does not match {{{ ^ }}} and the whitespace
 would have already been consumed by the previous group.

 One solution would be to remove the trailing-space-muncher.

 {{{
 (?:\s|^);(...variants...)
 or
 (?:\s|^):(...variants...)
 or
 (?:\s|^)8(...variants...)
 }}}

 The {{{ (?:\s|$) }}} clause never helped any of the ":" and ";" type
 smilies to be parsed only when surrounded by whitespace ({{{ :)text }}}
 will result in a parsed smilie, not no space after bracket, should this be
 parsed or not?) Noticed that the {{{ 8) }}} smilie is gone in the trunk
 due to it breaking regular text.

 Another solution would be to use lookaheads to test for trailing spaces,
 these do not consume the spaces themselves.

 {{{
 ((?:\s|^);(...variants...)(?=\s|$))
 or
 ((?:\s|^):(...variants...)(?=\s|$))
 or
 ((?:\s|^)8(...variants...)(?=\s|$))
 }}}

 This solves {{{ :)text }}}, if it is a problem.

 {{{
 (?:\s|^);(...variants...)
 or
 (?:\s|^):(...variants...)
 or
 (?:\s|^)8(...variants...)(?=\s|$)
 }}}

 Appears to be valid as well, only "8"-type smilies will expect tailing
 whitespace, but will not affect expected front-whitespace, fixing the
 combinations bug.

 I'm probably missing something but attached are very simple diffs then.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/20124#comment:2>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software