Make WordPress Core

Opened 11 years ago

Last modified 5 years ago

#23308 new defect (bug)

make_clickable problem with multiple "Punctuation URL character"

Reported by: drpepper75's profile DrPepper75 Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.1
Component: Formatting Keywords: has-patch 2nd-opinion
Focuses: Cc:


make_clickable problem with multiple "Punctuation URL character"


Results in this html code:

<a href="" rel="nofollow"></a>.html

But obvious should be:

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

I suggest to replace:

[\'.,;:!?)]  # Punctuation URL character


[\'.,;:!?)]{1,}  # Punctuation URL character

Attachments (1)

23308.diff (3.3 KB) - added by mdawaffe 10 years ago.

Download all attachments as: .zip

Change History (6)

#1 follow-up: @nacin
11 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Hi DrPepper75, thanks for this. Sorry it's taken so long for your bug report to get a response.

I tried out your suggestion. It broke another test, which is:

blah blah blah blah

When given )., it's much more likely for . to not be part of the URL, so that's probably the case we'd want to handle most often. That said, the test it broke struck me as a bit odd. If a space was before "moreurl", the test would pass both before and after the patch. I'll see if I can get someone else to look at this.

#2 @beaulebens
10 years ago

FWIW, I just came across what appears to be this same issue when trying to post a Github comparison URL in a comment. e.g. this URL gets the first chunk (up to 2.9b3) linked when make_clickable runs over it, but it chops off the ...2.9b4 part.

10 years ago

#3 in reply to: ↑ 1 @mdawaffe
10 years ago

Replying to nacin:

blah blah blah blah

That test is confusing. Why is the suffix "moreurl" if it's not meant to be in the URL?

We could do something like: attachment:23308.diff

  • Allow consecutive punctuation characters as long as they are followed by a non-punctuation character.

Passes all current and attached tests.

#4 @mdawaffe
10 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch needs-unit-tests removed

#5 @chriscct7
9 years ago

Patch still good to go

Note: See TracTickets for help on using tickets.