#23756 closed defect (bug) (fixed)
make_clickable should not convert the URLs in pre,code tags
Reported by: | alex-ye | Owned by: | wonderboymusic |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | 3.5.1 |
Component: | Formatting | Keywords: | has-patch needs-testing has-unit-tests commit |
Focuses: | Cc: |
Description
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 :)
Attachments (8)
Change History (34)
#3
follow-up:
↓ 4
@
12 years ago
does this function is used when displaying the post? -> the_content()
#5
in reply to:
↑ 4
;
follow-up:
↓ 7
@
12 years ago
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
#6
follow-up:
↓ 8
@
12 years ago
Making it skip <pre>
and <code>
tags might still be a good idea though, if it doesn't have a noticeable effect on performance.
#7
in reply to:
↑ 5
@
12 years ago
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 ..
#8
in reply to:
↑ 6
@
12 years ago
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 .
#12
@
11 years ago
- Keywords needs-unit-tests added
- Milestone changed from Awaiting Review to 3.7
Nice. This was more difficult to implement until we started splitting on HTML elements.
Needs unit tests!
#13
follow-up:
↓ 14
@
11 years ago
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
#14
in reply to:
↑ 13
@
11 years ago
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 insidepre
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
#15
@
11 years ago
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().
#16
@
11 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
- Milestone changed from 3.7 to 3.8
Moving to 3.8 for review and further discussion.
#17
@
11 years ago
What about attributes assigned to <code> or <pre>? We should account for that possibility as well.
#18
@
11 years ago
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.
#19
@
11 years ago
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>
#21
@
11 years ago
- Keywords commit added
I'd prefer $nested_code_pre > 0
rather than the implicit check, but that's just me. +1 otherwise.
#22
@
11 years ago
- Owner set to wonderboymusic
- Resolution set to fixed
- Status changed from new to closed
In 26052:
#23
@
11 years ago
- Cc bpetty added
- Resolution fixed deleted
- Status changed from closed to reopened
For good measure, added in case insensitive modifier for <pre>
and <code>
tag searches in ticket-23756-ignore-case.patch
#25
@
11 years ago
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
:-)
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 :)