milesj / decoda

A lightweight lexical string parser for BBCode styled markup.
MIT License
196 stars 52 forks source link

Emoticons are not parsed within quote tag #75

Closed ErikMinekus closed 10 years ago

ErikMinekus commented 10 years ago

Code:

$code = new Decoda\Decoda('[quote=milesj]Hello, my name is [b]Miles Johnson[/b] :)[/quote] [b]Hello[/b] ;)');
$code->defaults();
$code->addHook(new \Decoda\Hook\EmoticonHook());
echo $code->parse();

Output:

<blockquote class="decoda-quote">
    <div class="decoda-quote-body">
        Hello, my name is <b>Miles Johnson</b> :)
    </div>
</blockquote>
<b>Hello</b> <img src="/images/wink.png" alt="">

As you can see the :) inside the quote is rendered as text instead of an image, which seems to have to do with persistContent being false in QuoteFilter.

milesj commented 10 years ago

Emoticons need whitespace on the left and right side to parse correctly. Try placing a space after the smiley to see if it works.

https://github.com/milesj/decoda/blob/master/src/Decoda/Hook/EmoticonHook.php#L87

This is a hard requirement so that smileys don't convert proper characters, like :/ in http:/.

ErikMinekus commented 10 years ago

Ah I see, that works. Make sense, thanks!

ErikMinekus commented 10 years ago

Sorry, maybe I was a bit too hasty. Of course I already have a lot of content which doesn't have a space or newline before [/quote], so isn't it possible to add the closing bracket to the left and the opening bracket to the right of the regex? Or would that give too many false positives?

milesj commented 10 years ago

That might be a viable solution. Could you try applying it to all your examples? See if any issues arise?

alquerci commented 10 years ago

Should be tested with smile contains opening and/or closing bracket.

milesj commented 10 years ago

Yes of course, but tests only cover small snippets. Applying it to a large set of posts with complex BB code could help reveal weird edge cases.

milesj commented 10 years ago

Having no luck with this. I tried adding [] to the regex pattern with no luck. It seems the $smiliesRegex is causing conflicts.

https://github.com/milesj/decoda/blob/master/src/Decoda/Hook/EmoticonHook.php#L87

This is the regex I tried: $pattern = sprintf('/(?P<left>^|\n|\s|\])(?:%s)(?P<right>\[|\n|\s|$)/is', $smiliesRegex);

ErikMinekus commented 10 years ago

I got it to work by making $smiliesRegex a capturing group:

        $pattern = sprintf('/(?P<left>^|\n|\s|\%s)(?P<smiley>%s)(?P<right>\%s|\n|\s|$)/is',
            $this->getParser()->getConfig('close'),
            $smiliesRegex,
            $this->getParser()->getConfig('open'));

And then in _emoticonCallback I used that instead of $matches[0]:

        $smiley = trim($matches['smiley']);

But I have not yet had the time to test it full scale.

milesj commented 10 years ago

Cool, I'll give that a try. I only spent like 5 minutes on it before getting distracted, hah.

ErikMinekus commented 10 years ago

I have not found any issues with this yet. I tested it on a bunch of posts, and with things like:

[quote] :cool:[/quote]
[quote] :)[/quote]
[quote] :[[/quote]
[quote] :[/[/quote]
[quote]]:[/[/quote]

And it's all working.

milesj commented 10 years ago

Thanks to @alquerci and I, we pushed some changes up. Want to give master a shot before I tag it?

ErikMinekus commented 10 years ago

Seems to be working great. Stuff like [quote]]:[/[/quote] does not work anymore, but that's fine, it would give too many false positives.

milesj commented 10 years ago

Yeah, don't believe I would want to support such invalid syntax anyways, hah.

milesj commented 10 years ago

Thanks to @alquerci I believe we have something working. Can you re-try again with master? https://github.com/milesj/decoda

ErikMinekus commented 10 years ago

Thanks a lot, still works well. I tested with this crazy text:

[b] :([/b]

[QUOTE] ];[QUOTE] :/[QUOTE]Some text[/QUOTE]

:[/[/QUOTE]

More [i]text[/i];)
:[[/QUOTE]

All emoticons were parsed correctly.

milesj commented 10 years ago

Perfect! I'll close this and tag a version soon.