Skip to:
Content

bbPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#3237 closed defect (bug) (fixed)

bbp_make_clickable() doesn't handle spaces in linked text, and incorrectly duplicates `<a>`

Reported by: dd32's profile dd32 Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.2
Component: API - Formatting Keywords: needs-patch
Cc:

Description (last modified by dd32)

As reported in https://meta.trac.wordpress.org/ticket/3998

<a href="https://codex.wordpress.org/Roles and Capabilities">https://codex.wordpress.org/Roles and Capabilities</a>

turns into:

<a href="https://codex.wordpress.org/Roles and Capabilities" rel="nofollow"></a><a href="https://codex.wordpress.org/Roles" rel="nofollow">https://codex.wordpress.org/Roles</a> and Capabilities

Looks like either make_clickable() or bbp_make_clickable() is trying to make the URL clickable without taking the existing <a> tag into account.

As I've commented on the ticket:


This is a problem in bbp_make_clickable(), but is also present in make_clickable().

Given the input string of <a href="https://codex.wordpress.org/Roles and Capabilities">https://codex.wordpress.org/Roles and Capabilities</a> both will return the invalid output.

Both contain the following to correct it:

return preg_replace( '#(<a([ \r\n\t]+[^>]+?>|>))<a [^>]+?>([^>]+?)</a></a>#i', "$1$3</a>", $r );

But as the resulting HTML is the following it's not matched (due to the </a></a> - which assumes that neither the linked text never has spaces or isn't a URL)

<a href="https://codex.wordpress.org/Roles and Capabilities"><a href="https://codex.wordpress.org/Roles" rel="nofollow">https://codex.wordpress.org/Roles</a> and Capabilities</a>

Adjusting the regular expression to the following does work however:

return preg_replace( '#(<a([ \r\n\t]+[^>]+?>|>))<a [^>]+?>([^>]+?)</a>([^<]*)</a>#i', "$1$3$4</a>", $r );

As a work around, you can remove the spaces from the linked text, which avoids it:
<a href="https://codex.wordpress.org/Roles and Capabilities">https://codex.wordpress.org/Roles&nbsp;and&nbsp;Capabilities</a>.


I've also created https://core.trac.wordpress.org/ticket/45702

Change History (5)

#1 @dd32
6 years ago

  • Description modified (diff)

#2 @johnjamesjacoby
6 years ago

  • Component changed from General to API - Formatting
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.6
  • Owner set to johnjamesjacoby
  • Version set to 2.2

#3 @netweb
6 years ago

  • Keywords needs-patch added; has-patch removed

#4 @johnjamesjacoby
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In 6885:

Formatting: update bbp_make_clickable() to better handle spaces in href attributes.

This change updates the included regular expression to avoid returning broken (or at least unexpected) HTML and allows spaces in href attributes to be encoded as expected.

Props dd32. Fixes #3237.

#5 @dd32
6 years ago

Just want to note that my proposed solution will break if there's a URL and HTML in the linked text, but that's even more edge-casey than the reported problem:

<a href="https://testing/path/">http://testing/<strong>path</strong>/</a>

becomes

<a href="https://testing/path/"><a href="http://testing/" rel="nofollow">http://testing/</a><strong>path</strong>/</a>

The regex in question isn't designed to prevent every case, just the most obvious ones IMHO.

A better fix, could be to skip linking inside of <a> tags through the same logic applied for <script, style, code, and pre> tags above it: https://bbpress.trac.wordpress.org/browser/trunk/src/includes/common/formatting.php?rev=6885&marks=365-374#L344

Note: See TracTickets for help on using tickets.