Hi,

I'm facing a problem when I've some <br /> tags in a paragraph dom node.

I think the problem is in _glossify_replace function (around line 366 of glossify.module).
The point is that when looping on child nodes, if there are child nodes there's a recursion and on textNodes children the innertext property is lost. To avoid this, when the child is a TextNode, there should be a nodeValue or textContent instead of an innertext.

For instance, if you have :

<html>
<p>
Some Text
</p>
</html>

There will be no problem since P node doesn't have children and "Some Text" will be its innertext.
But if you have this :

<html>
<p>
Some Text,
<br />
Some other text.
</p>
</html>

Then P node will have 3 children, and TextNodes children won't have an associated tag, and no innerText, so you will never pass through the replacement instruction (around line 375 of glossify.module), even if in the text you have patterns to replace by links.

Am I the only one facing this problem?

Regards,
cfab

Comments

tahiticlic’s picture

tahiticlic commented

Well, this is not a solution, but adding 

        // strip br
        $this->remove_noise("'<br />'is", true);

to modules/simplehtmldom/simplehtmldom/simple_html_dom.php in load function around line 545 solves the problem.

Ivo.Radulovski’s picture

Ivo.Radulovski commented
Status: Active » Postponed

Hi and thanks for your interest in Glossify, no you aren't the only one facing this problem, i have been working on fixing this issue last week but i got other stuff in between so i wasn't able to get anything out, i also recieved patches and other suggestions which i will try to implement asap, it's just that my desk is full with other things as well right now.

tahiticlic’s picture

tahiticlic commented

Okay, I began to feel a bit lonely! ;-)

Well, I guess this should be a better solution impacting only Glossify and not the simple dom library... Anyway, this is a temporary solution at least!

Ivo.Radulovski’s picture

Ivo.Radulovski commented

Please give the next beta4 a try and see if it fixes your problems.
Thanks for your interest in Glossify.

tahiticlic’s picture

tahiticlic commented

Hi,

the problem is still here even with the 3b4 version.

Thanks for trying to solve!

Ivo.Radulovski’s picture

Ivo.Radulovski commented
Status: Postponed » Needs review

I'm sorry it looks like the beta4 for some reason didn't correctly upload in December so I would like to ask you one more time to give the beta5 a try (actually beta4).
Sorry for the inconvenience but I'm fairly certain this release will fix your issue.

Ivo.Radulovski’s picture

Ivo.Radulovski commented

actually beta8 - had to update again :)

Ivo.Radulovski’s picture

Ivo.Radulovski commented
Version: 6.x-3.0-beta3 » 6.x-3.0-beta8
Ivo.Radulovski’s picture

Ivo.Radulovski commented
Version: 6.x-3.0-beta8 » 6.x-3.0-beta9

you now can exclude or include HTML tags that shouldn't be filtered by Glossify

tahiticlic’s picture

tahiticlic commented

Great! thanks!

Ivo.Radulovski’s picture

Ivo.Radulovski commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.