Django

Code

Ticket #7542 (closed: fixed)

Opened 2 months ago

Last modified 2 months ago

urlize changing link text as well as url

Reported by: devin Assigned to: devin
Milestone: Component: Template system
Version: SVN Keywords:
Cc: devin@disqus.com Triage Stage: Unreviewed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

The autoescaping introduced in [7079] had the unintended side-effect of appending http:// to both the link target and text instead of just the target.

eg:

>>> urlize('google.com')
u'<a href="http://google.com">http://google.com</a>'

Instead of the pre-[7079] behavior of:

>>> urlize('google.com')
u'<a href="http://google.com">google.com</a>'

Tests weren't written until [7701], so this wasn't caught. But it popped up on our internal tests here at Disqus.

Since urlize is intended just to convert text to clickable links, not change what the text says, this is undesirable. This is mostly used for user generated content, and I feel it's wrong to be changing what the user typed.

I've attached a patch that fixes that problem. We need to keep a url target and a link text separate. Besides, I've cleaned up the function a bit as it got largely unreadable after [7079] and updated the docstring to include mention of autoescape and the fact that this function now converts urls ending in .org, .net and .com. As well as updating the tests included in [7701] to match this expected functionality.

Attachments

simple_urlize_diff.diff (5.1 kB) - added by devin on 06/25/08 23:43:18.
fixes urlize

Change History

06/25/08 23:43:18 changed by devin

  • attachment simple_urlize_diff.diff added.

fixes urlize

06/25/08 23:53:21 changed by Alex

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

Line 94 uses the ternary syntax(a if condition else b), which is a Python 2.5ism, Django maintains compatibility with versions from 2.3 to 2.5 so this should be changed, I haven't had a chance to look at anything else in this patch, that just stuck out at me.

06/26/08 00:07:14 changed by adrian

  • status changed from new to closed.
  • resolution set to fixed.

(In [7755]) Fixed #7542 -- Fixed bug in urlize where it was appending 'http://' to the link text. Thanks for the patch and tests, devin


Add/Change #7542 (urlize changing link text as well as url)




Change Properties
Action