MyBB Community Forums

It's an old issue: http://dev.mybb.com/issues/2099

I still don't see why it's a problem to parse the longest smiley first. I've patched my board to do so; it works just fine for me and fixes undesired conflicts between and () () ()

Sure, if you add : )) as a smiley and people used (something in parentheses) in the past, it would change that smiley and it would probably not be the desired result. But that's the same argument as the smilie aliases ( https://github.com/mybb/mybb/issues/902 ) - it's up to the board admin to pick proper smiley strings.

I guess we could apply your fix?

My fix? Oh well. In that bug report I suggested sorting the smiley cache by key length. Which works fine, and is the minimal change that retains current smiley parser semantics, but I've actually done something different.

PHP has this nice mass string replacement function strtr() that works exactly this way (longest first).

So instead of this:

foreach ($this->smilies_cache as $find => $replace)
{
    $orig_message = $message;
    $find = $this->parse_html($find);
    $find = preg_quote($find, "#");
    
    $replace = strip_tags($replace, "<img>");
    
    // Fix issues for smileys starting with a ";"
    $orig_find = $find;
    if (substr($find, 0, 1) == ";")
    {
        $find = "(?<!&gt|&lt|&amp)" . $find;
    }
    
    $message = @preg_replace("#(?<=[^\"])" . $find . "(?=.\W|\"|\W.|\W$)#si", $replace, $message, $remaining, $replacements);
    
    if ($message == null)
    {
        $message = preg_replace("#(?<=[^&;\"])" . $orig_find . "(?=.\W|\"|\W.|\W$)#si", $replace, $orig_message, $remaining);
    }
    
    $remaining -= $replacements;
    if ($remaining <= 0)
    {
        break; // Reached the limit
    }
}

I have this:

$message = strtr($message, $this->smilies_cache);

The upside is you don't have to sort and it makes smiley parsing faster by an order of magnitude.

The downside is you can't have custom rules like (?<=[^\"])".$find."(?=.\W|\"|\W.|\W$), smiley limit to 500, and whatever else that foreach loop is doing. Which is fine by me since I don't need it.

The // Fix issues for smileys starting with a ";" also isn't there. Normally, the only smiley that is affected by that is or ;-) and you can protect that in strtr() by adding "&amp;"=>"&amp;", "&gt;"=>"&gt;", "&lt;"=>"&lt;", to the smiley cache array. Since "&lt;" with 4 chars is longer than ";-)" with three chars, it will replace "&lt;" (with itself) first and or ;-) is protected if there is a &) anywhere.

strtr is a good function but It is Case Sensitive. :-\

do you use case sensitive smilies? I think almost all of them are not affected

Yes, it's case sensitive. I see this as an added bonus really, for example :-d and :-D are quite different visually.

So if you want two different cases for the same smiley, like :-P and :-p, you'd have to add them as aliases.

I don't think anyone does TUFF: instead of :stuff: for verbose style of smileys...

With the new alias functionality case sensitive replacements don't matter as much. We could, for example, update the : P smilie to add : p as an alias (no spaces) by default and in the upgrade process.
The alias upgrade routine finds smilies pointing to the the same image and merges them together into one smilie with aliases.

So, should I send a pull request?

I don't see why not

@frostschutz I think You should fix it for 1.6 too.


---
please remove [Rejected] lable :-\