WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 5 months ago

#23308 new defect (bug)

make_clickable problem with multiple "Punctuation URL character"

Reported by: DrPepper75 Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.1
Component: Formatting Keywords: has-patch 2nd-opinion
Focuses: Cc:
PR Number:

Description

make_clickable problem with multiple "Punctuation URL character"

E.g.

http://www.wordpress.org/some-(parentheses).html

Results in this html code:

<a href="http://www.wordpress.org/some-(parentheses)" rel="nofollow">http://www.wordpress.org/some-(parentheses)</a>.html

But obvious should be:

<a href="http://www.wordpress.org/some-(parentheses)" rel="nofollow">http://www.wordpress.org/some-(parentheses).html</a>

I suggest to replace:
wp-includes/formatting.php:1603

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

with

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

Attachments (1)

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

Download all attachments as: .zip

Change History (6)

#1 follow-up: @nacin
6 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 http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).)moreurl 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
6 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.

https://github.com/Automattic/jetpack/compare/2.9b3...2.9b4

@mdawaffe
5 years ago

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

Replying to nacin:

blah blah http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).)moreurl 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
5 years ago

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

#5 @chriscct7
4 years ago

Patch still good to go

Note: See TracTickets for help on using tickets.