Opened 5 years ago
Closed 4 years ago
#8300 closed defect (bug) (fixed)
Add parentheses to the list of valid URL characters for make_clickable function
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | normal | Milestone: | 2.8 |
| Component: | General | Version: | 2.7 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | josephscott |
Description
The make_clickable currently doesn't work correctly for URLs with parentheses like these:
http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software) http://msdn.microsoft.com/en-us/library/aa752574(VS.85).aspx
By adding () to the list of valid URL characters these URLs are correctly detected. I've included a simple patch to do just that.
Attachments (4)
Change History (15)
josephscott
— 5 years ago
comment:2
in reply to:
↑ 1
josephscott
— 5 years ago
Replying to DD32:
You need to be careful about including parentheses in function to make links, As (http://blah.com/) should not be matched as a link, only the insides, yet http://blah.com/something(else)_another should..
There are some link parsing regular expressions around which deal with those cases specifically, somethin more robust might be required.
Just so happens that I just wrote some code to deal with this, so that this:
My site (http://josephscott.org)
So that the the open and closing parens are not included as part of the URL, will still supporting parens inside the URL.
I've updated the patch to reflect this.
josephscott
— 5 years ago
comment:3
filosofo
— 5 years ago
Actually, you can do this without having to modify _make_url_clickable_cb: just change the regex to capture a closing parenthesis at the end of the URL only if the entire URL is not preceded by an opening parenthesis.
make_clickable_parenthesis.diff gets all of the following examples right:
http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software) http://msdn.microsoft.com/en-us/library/aa752574(VS.85).aspx My site (http://josephscott.org)
comment:4
filosofo
— 5 years ago
Actually, it occurred to me that we could make _make_url_clickable_cb even shorter, using the same regex technique to remove ending .,;:, as seen in patch leaner_make_url_clickable_cb.diff
comment:5
josephscott
— 5 years ago
I tried the changes in leaner_make_url_clickable_cb.diff and it failed to pick up the closing paren for this test case:
My (http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)) article
That got converted to:
My (<a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software))" rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software))</a> article
That last ) should have been outside the </a> tag.
comment:7
josephscott
— 5 years ago
Nicely done, that fixes the previous test case problem.
comment:8
josephscott
— 5 years ago
After more tests it turns out that the new regular expression doesn't work in PHP4. Something changed in PHP5 preg_replace_callback() that allow it to work.
comment:10
josephscott
— 5 years ago
Sorry, this looks like a false alarm. Patch works on PHP4 as well.
comment:11
automattor
— 4 years ago
- Resolution set to fixed
- Status changed from new to closed
You need to be careful about including parentheses in function to make links, As (http://blah.com/) should not be matched as a link, only the insides, yet http://blah.com/something(else)_another should..
There are some link parsing regular expressions around which deal with those cases specifically, somethin more robust might be required.