milesj / decoda

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

Wrong parse result for [image] #70

Closed hilobok closed 10 years ago

hilobok commented 10 years ago

Following BBCode

[image]/some/url.png[/image]

should be parsed as

<img src="/some/url.png alt="">

but it parsed to instead

<img src="/some/url.png" alt="">/some/url.png</img>

At the same time [img] tag works as supposed.

milesj commented 10 years ago

What version? image and img are the same thing. https://github.com/milesj/decoda/blob/master/src/Decoda/Filter/ImageFilter.php

hilobok commented 10 years ago

I'm using latest version — 6.5.0.

Yes it's the same thing, but merging of properties for aliased tags not working properly, some of properties are overwritten with defaults here.

In case with image alias autoClose option overwritten, which leads to render not self-closing tag.

I've quick fixed this with this piece of code instead of mentioned:

            // Alias tags and inherit from another tag and merge recursively
            if ($filter['aliasFor']) {
                // copy options from parent besides 'tag' and 'aliasFor' options
                $base = array_diff_key(
                    $tags[$filter['aliasFor']],
                    array('tag' => '', 'aliasFor' => '')
                );

                foreach ($filter as $key => $value) {
                    // do not overwrite options from inherited tag
                    if (isset($base[$key]) && !is_array($base[$key])) {
                        continue;
                    }

                    if (is_array($value)) {
                        $base[$key] = $value + $base[$key];
                    } else if ($value !== '') {
                        $base[$key] = $value;
                    }
                }

                $filter = $base;
            }

But it's not fully correct as for me.

hilobok commented 10 years ago

Also you can test this behavior by adding test:

    public function testImageAlias() {
        $this->assertEquals('<img src="/some/url.png" alt="">', $this->object->reset('[image]/some/url.png[/image]')->parse());
    }
hilobok commented 10 years ago

I also suspect all other aliased tags exposed to this bug.

milesj commented 10 years ago

I see what you mean. The problem is the $defaults being merged in. Should maybe not do that for aliased fields.

hilobok commented 10 years ago

Sounds reasonable, could you fix this please. I'm stuck before rolling to production because of this bug :)

milesj commented 10 years ago

Yeah, I'll try to get around to it EOD.

milesj commented 10 years ago

Should be fixed.