WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 6 months ago

#23922 new defect (bug)

make_clickable() breaks when colon in hash

Reported by: Viper007Bond Owned by:
Milestone: Future Release Priority: lowest
Severity: minor Version:
Component: Formatting Keywords: needs-patch has-unit-tests
Focuses: Cc:

Description

make_clickable() doesn't like this string:

<a href="http://en.wikipedia.org/wiki/URI_scheme#tel:">http://en.wikipedia.org/wiki/URI_scheme#tel:</a>

It results in this HTML:

<a href="http://en.wikipedia.org/wiki/URI_scheme#tel:"><a href="http://en.wikipedia.org/wiki/URI_scheme#tel" rel="nofollow">http://en.wikipedia.org/wiki/URI_scheme#tel</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 9 months ago.
Add some tests for a couple of specific hash conditions.

Download all attachments as: .zip

Change History (7)

comment:1 nacin15 months ago

  • Keywords needs-unit-tests added

Well, colons are a reserved character and invalid in URLs: http://www.blooberry.com/indexdot/html/topics/urlencoding.htm. 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.

comment:2 Viper007Bond15 months 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.

comment:4 alex-ye15 months ago

  • Cc nashwan.doaqan@… added

in related to make_clickable() , could you see this ticket, please ?
http://core.trac.wordpress.org/ticket/23756

Version 0, edited 15 months ago by alex-ye (next)

ericmann9 months ago

Add some tests for a couple of specific hash conditions.

comment:5 ericmann9 months 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.

comment:6 nacin6 months ago

  • Milestone changed from Awaiting Review to Future Release
Note: See TracTickets for help on using tickets.