make_clickable should not convert the URLs in pre,code tags

[alex-ye]

Hi , Maybe it's a duplicate ticket but I tried to search in history.

Anyway , The purpose of this ticket is to have a clean snippet code posts .. for now it's hard to do it by plug-in .. and many people remove make_clickable filter at all !

So let's make it possible in 3.6 :)

[alex-ye]

and who will say that I must escape the URLs before posting them so the make_clickable won't work , It's really hard and dirty solution !!

Many users don't do that and many users facing problems !! , I think we should not convert the URLs in pre,code tags at all !! , or at least make it easy by some hooks :)

[alexvorn2]

does this function is used when displaying the post? -> the_content()

[alex-ye]

Replying to alexvorn2:

does this function is used when displaying the post? -> the_content()

Yes

[SergeyBiryukov]

Replying to alex-ye:

Replying to alexvorn2:

does this function is used when displaying the post? -> the_content()

Yes

No, it isn't: #18214.

The purpose of this ticket is to have a clean snippet code posts

make_clickable() is only applied to comment_text() by default:
​http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/default-filters.php#L151

[SergeyBiryukov]

Making it skip <pre> and <code> tags might still be a good idea though, if it doesn't have a noticeable effect on performance.

[alex-ye]

Replying to SergeyBiryukov:

No, it isn't: #18214.

Thank you for correction , I make sure that a plugin who adds the make_clickable to the_content filter ..

[alex-ye]

Replying to SergeyBiryukov:

Making it skip <pre> and <code> tags might still be a good idea though, if it doesn't have a noticeable effect on performance.

Yes , I face problems in comments or topics/replies also in bbPress , I made my own hack to fix this but I think it is something the core should do .

The bug be bigger when you use a Syntax Highlighter plugins .

[johnjamesjacoby]

Skip links in code and pre blocks.

[nacin]

Nice. This was more difficult to implement until we started splitting on HTML elements.

Needs unit tests!

[helen]

I WROTE A UNIT TEST YOU GUYS. 23756.test.diff​

Looking at jjj's patch, it seemed to me that nesting them would break things, as in it would end the $in_code flag early, which does seem to be the case. The last test fails; however, I'm not sure that you're supposed to have a code element inside pre unless it wraps everything as well, e.g. <pre><code>blah</code></pre>. See ​http://www.w3.org/TR/html5/grouping-content.html#the-pre-element

[adamsilverstein]

add nest level count, corrects issue with nested pre & code tags

[adamsilverstein]

Replying to helen:

I WROTE A UNIT TEST YOU GUYS. 23756.test.diff​

Looking at jjj's patch, it seemed to me that nesting them would break things, as in it would end the $in_code flag early, which does seem to be the case. The last test fails; however, I'm not sure that you're supposed to have a code element inside pre unless it wraps everything as well, e.g. <pre><code>blah</code></pre>. See ​http://www.w3.org/TR/html5/grouping-content.html#the-pre-element

your unit test looks good to me. i added a nested level count in 23756.2.diff​​ and your test now passes! not sure pre tag surrounding code is right, but it is going to happen, so makes sense to account for the possibility. even in the w3.org page you linked they have nested that way in their sample code. ​http://cl.ly/Qp6K

[nacin]

23756.3.diff​ merges the tests with the code and tidies things up a bit.

It looks good to me, but I am asking that duck_ or mdawaffe review this as well given their experience with the function and potential security implications (XSS) with make_clickable().

[kpdesign]

Moving to 3.8 for review and further discussion.

[nacin]

What about attributes assigned to <code> or <pre>? We should account for that possibility as well.

[sirbrillig]

Updated combined patch and test that covers code and pre tags with attributes.

[sirbrillig]

I updated the test to include code and pre tags with attributes as well as custom tags that start with the words 'code' or 'pre'. This caused the test to fail. I then updated the function part of the patch to pass the test.

23756.5.diff​

[sirbrillig]

Re-added @ to ticket number in test.

[kpdesign]

I've been watching this ticket, as this is an issue that affects the handbooks. The handbooks use the ​SyntaxHighlighter Evolved plugin, and all of the code samples containing URLs are broken (they display full markup for a URL instead of just the URL). See ​Automated Testing > Installation for an example.

I applied 23756.5.diff​ to my local install (latest trunk), tested, and got the expected results listed in the unit test.

This patch also fixed my local test code samples using the shortcodes in the plugin (no markup added to URLs).

Code:

[sourcecode lang="bash"]
$ URL https://core.trac.wordpress.org/attachment/ticket/00000/00000.patch
$ Email name@domain.tld
$ FTP ftp://user:password@domain.tld/path
[/sourcecode]

Generated markup:

<pre class="brush: bash; title: ; notranslate" title="">
$ URL https://core.trac.wordpress.org/attachment/ticket/00000/00000.patch
$ Email name@domain.tld
$ FTP ftp://user:password@domain.tld/path
</pre>

[rmccue]

I'd prefer $nested_code_pre > 0 rather than the implicit check, but that's just me. +1 otherwise.

[wonderboymusic]

In 26052:

Don't convert URLs inside <pre> and <code> tags when parsing string using make_clickable().
Adds Unit Tests.

Props johnjamesjacoby, helen, nacin, adamsilverstein, sirbrillig.
Fixes #23756.

[bpetty]

For good measure, added in case insensitive modifier for <pre> and <code> tag searches in ticket-23756-ignore-case.patch​

[wonderboymusic]

In 26094:

Make HTML tag searches for <code> and <pre> case-insensitive in make_clickable().

Props bpetty.
Fixes #23756.

[nbachiyski]

There is some prior art of not mangling certain parts of the HTML in wptexturize(). Look at the bottom of the function. It handles more cases – some shortcodes (code), more tags (kbd, style, script, tt), handles some invalid HTML cases, supports filtering the disabled tags/shortcodes.

It’d be great if the logic for the two can be consolidated, because for example now make_clickable will convert URLs in script tags and in shortcodes.

if anybody else is excited, I’d be happy to help.

P.S. I wrote the older code, but I’m pretty embarrassed of functions like _wptexturize_pushpop_element :-)