I noticed that the widont feature operates on the last line of code snippets put inside <pre> </pre> tags so I had a look at the implementation and noticed that the regular expression used is incorrect.

Please find attached two files:

  • php-typogrify.fix_.patch provides my personal fix to the current implementation
  • php-typogrify.new_.patch patches the current implementation to the version found at http://wordpress.org/extend/plugins/wp-typogrify/ (possibly more complete and up to date)

patching against the newer implementation seems to work for me, otherwise you can stick to the implementation the module currently uses and only apply the widont fix

cheers

CommentFileSizeAuthor
#9 typogrify-widont.patch1.33 KBneochief
#4 test.txt8.4 KBneochief
php-typogrify.new_.patch19.3 KBguardian
php-typogrify.fix_.patch1.52 KBguardian

Comments

guardian’s picture

guardian commented
Title: Widont fails to bypass text inside <pre></pre> tags » Widont treats </pre> as </p> and alters preformatted text
Status: Active » Needs review

more accurate title and status

guardian’s picture

guardian commented
Title: Widont treats </pre> as </p> and alters preformatted text » Widont treats &lt;pre&gt; as &lt;/p&gt; and alters preformatted text
neochief’s picture

neochief commented

I can say that newer widont implementations breaks pages on my site. If you have a big article, complex regexp which widont contains overflows preg's stack. It's not hapening when I'm using older version. Maybe python has a better regexp implementation, but php sucks in this case.

neochief’s picture

neochief commented
Status: Needs review » Needs work
StatusFileSize
new test.txt8.4 KB

In widont($text) the preg_replace($widont_finder, '$1&nbsp;$2', $text); returns NULL in my case. The preg_last_error(); returns PREG_BACKTRACK_LIMIT_ERROR.

By playing with that big regexp in widont(), I found that if you'll remove [^\s<>]* inside it, everything begins to work fine. Actually, I simply don't understant what they trying to match with this thing (" <>" ????), but it's very likelly that the devil sits in it.

I've attached a content on which is happens.

mikl’s picture

mikl
Location Møn
commented
Assigned: Unassigned » mikl

Okay, that change seems sensible. I'll give it a shot.

guardian’s picture

guardian commented

is this fix applied in 5.x-1.0-beta4 please?

mikl’s picture

mikl
Location Møn
commented

No, I've been trying to put some test cases together, so I can try and refactor some of the heavy RegExes in Typogrify without creating new bugs – sorry for the delay…

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I think I ran into this on v6.x, I posted an article with heavy use of PRE and CODE only to have the entire body disappear when the Typogrify filter was enabled.

neochief’s picture

neochief commented
Title: Widont treats &lt;pre&gt; as &lt;/p&gt; and alters preformatted text » Widont treats <pre> as </p> and alters preformatted text
Version: 5.x-1.x-dev » 6.x-1.x-dev
Status: Needs work » Needs review
StatusFileSize
new typogrify-widont.patch1.33 KB

Here's a patch with which my site works fine about a 3 month so far. It's against latest 6.x

mikl’s picture

mikl
Location Møn
commented
Status: Needs review » Fixed

All right, I've committed the patch from #9, going to roll a beta 5 release soon :)

mikl’s picture

mikl
Location Møn
commented

Hmm, it would seem that the patch from #9 introduces a regression – the nbsp doesn't replace the space, but is added, giving in effect two spaces.

I've written some more tests, so now I need to figure out a way to fix one without breaking the other…

mikl’s picture

mikl
Location Møn
commented
Status: Fixed » Needs work
mikl’s picture

mikl
Location Møn
commented
Status: Needs work » Fixed

The regression was fixed in #447416: Widont not working properly – thanks dboulet

Status: Fixed » Closed (fixed)

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