Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#17097 closed defect (bug) (invalid)

make_clickable() regex should exclude close-quotation marks after URLs

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.1
Component: General Keywords: close
Focuses: Cc:

Description

Originally reported in http://trac.buddypress.org/ticket/2443

The regex in make_clickable() thinks that a single-quote or a double-quote at the end of a URL string is part of the URL. Steps to reproduce:
1) Write a post:

The websites 'http://wordpress.org' and "http://buddypress.org" are awesome

2) Add the make_clickable filter:

add_filter( 'the_content', 'make_clickable' );

3) Output (html source):

The websites &#8216;<a href="http://wordpress.org&#038;#8217" rel="nofollow">http://wordpress.org&#038;#8217</a>; and &#8220;<a href="http://buddypress.org&#038;#8221" rel="nofollow">http://buddypress.org&#038;#8221</a>; are awesome

(front end)

The websites ‘http://wordpress.org&#8217; and “http://buddypress.org&#8221; are awesome

I would expect that, in the majority of cases, URLs will not end in a double- or single-quote. So it makes sense for the regex to count the quote as part of the after-URL backreference. See #5081 for a similar issue involving periods.

I'm afraid I'm not providing a patch because my puny mind cannot understand the expression currently being used :)

Change History (5)

#1 @greuben
13 years ago

  • Keywords close added

You are running the filter after add_filter( 'the_content', 'wptexturize' ); which converts the end ' to &#8217;. That is why make_clickable includes the '(&#8217). Set the priority of the filter to less than 10, you won't have that problem.

#2 @boonebgorges
13 years ago

Good call, greuben - that worked.

But now, a related issue: should make_clickable() be smart enough not to break up the HTML code for one of these special characters? Aside from a whitelist? This is definitely an edge case, but something to consider.

#3 @nacin
13 years ago

  • Keywords needs-patch removed

http://example.com?foo=bar&#8217 looks like a valid URL to me. make_clickable runs on comment_text at 9, just before wptexturize. I would consider that to be a requirement for its use. Or do I not fully understand your related issue?

#4 @boonebgorges
13 years ago

  • Resolution set to invalid
  • Status changed from new to closed
 http://example.com?foo=bar&#8217 looks like a valid URL to me.

Yeah, I guess that's right. Though my related issue is that, given the input string http://example.com?foo=bar&#8217;, which of the following outcomes is more likely to be expected:

<a href="http://example.com?foo=bar&#8217">http://example.com?foo=bar&#8217</a>;

or

<a href="http://example.com?foo=bar&#8217;">http://example.com?foo=bar&#8217;</a>

(FWIW, Trac is doing the same thing as WP here and excluding the semi-colon. So I'm probably just wrong.)

make_clickable runs on comment_text at 9, just before wptexturize. I would consider that to be a requirement for its use.

I'm referring to the use of make_clickable in a plugin, outside of comment_text. So I guess the answer is probably just that I'm doing it wrong, and should make sure that it runs before wptexturize. Thanks for your help.

#5 @nacin
13 years ago

  • Milestone Awaiting Review deleted

No problem, Boone. Yeah, I was just using comment_text as an example -- my point was that it explicitly runs just before wptexturize in core, so that seems like that's how it should be expected to be used by plugins.

Note: See TracTickets for help on using tickets.