Make WordPress Core

Opened 11 years ago

Last modified 5 years ago

#23922 new defect (bug)

make_clickable() breaks when colon in hash

Reported by: viper007bond's profile Viper007Bond Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Formatting Keywords: needs-patch has-unit-tests
Focuses: Cc:


make_clickable() doesn't like this string:

<a href=""></a>

It results in this HTML:

<a href=""><a href="" rel="nofollow"></a>:</a>

Specifically it's the colon that is causing the issue. It can be a part of the URL too, it doesn't have to be a part of an anchor.

Attachments (1)

23922.tests.diff (1.1 KB) - added by ericmann 11 years ago.
Add some tests for a couple of specific hash conditions.

Download all attachments as: .zip

Change History (8)

#1 @nacin
11 years ago

  • Keywords needs-unit-tests added

Well, colons are a reserved character and invalid in URLs: But they're not uncommon in hashes. Trac uses them, for comments (#comment:0). I'd be inclined to allow colons when used in a hash. But not sure this is a defect.

#2 @Viper007Bond
11 years ago

  • Summary changed from make_clickable() breaks when colon in path to make_clickable() breaks when colon in hash

Ah, okay. I fixed the ticket title then.

Really it should ignore anything inside of an existing <a> tag (note it's making a hyperlink's text clickable) but I'm not sure how feasible that is while maintaining good regex performance.

#4 @alex-ye
11 years ago

  • Cc nashwan.doaqan@… added

in related to make_clickable() , could you see this ticket, please ? #23756

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

11 years ago

Add some tests for a couple of specific hash conditions.

#5 @ericmann
11 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Added a test method to check three potential URL structures:

  1. Url with trailing colon (used in ticket description)
  2. Url with embedded colon in hash (used in Trac URLs specifically)
  3. Url with trailing period

Finding parity between the first and third cases is the difficulty here. The original make_clickable() method specifically only allows punctuation characters when followed by non-punctuation characters (meaning case #2 will actually work). Removing the colon from this punctuation check will allow case #1 to pass, but then case #2 will fail.

Whatever fix is introduced, retaining the functionality of case #3 (trailing non-colon punctuation) is vital to allowing links that are made clickable to be placed at the end of a sentence, before a coma, etc.

#6 @nacin
10 years ago

  • Milestone changed from Awaiting Review to Future Release

#7 @chriscct7
9 years ago

  • Priority changed from lowest to normal
  • Severity changed from minor to normal
Note: See TracTickets for help on using tickets.