425508 - Automagically create links from urls in user-entered text

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, enhancement)

Description

[Ryan Doherty (:rdoherty)]

Would be nice for links to be automagically created from urls that users enter into addon descriptions and reviews.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

The question is if all user-entered text should automatically linkify. If some standard AMO developer could tell me which fields should automagically be linkified, I will provide a patch.

Comment 2

[Wil Clouser [:clouserw]]

Addon.description sounds good to me.  Since reviews are moderated, we could probably get away with Review.body too.

Comment 3

[Fred Wenzel [:wenzel]]

I suggest handling this in the views only that need this, for example by adding a function linkify() to the html helper. I'd advice against doing this globally or, even worse, by storing HTML tags to the database.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Add Links to different fields

I am defintively using a helper. The Text helper class has functions to automatically do this. I must admit though that the cakephp autoLinkUrls functions is a bit lacking: A . or , at the end of an url (i.e in a sentance) will be part of the url. I think this should be fixed in cakephp though.

I created this patch with some more fields that I think might deserve autolinking. For example version notes might contain bug links or such. Go ahead and decide which fields should be taken.

Comment 5

[Fred Wenzel [:wenzel]]

Comment on attachment 313049 [details] [diff] [review]
Add Links to different fields

While I like the idea, I see two problems here:

- URLs are being over-escaped here. I see HTML entities in the link text.
- We need to decide if we want to use rel=nofollow in order to discourage spammy external comments, mainly due to the fact that most of the linkified fields are not moderated at all.

Comment 6

[Justin Scott [:fligtar]]

The reason this isn't a simple fix and hasn't been done already (bug 343573) is that we also need to use a different domain that redirects to URLs because if the link is actually an XPI, it will bypass the Firefox AMO whitelist for extension installs.

We could put something on services.amo, like services.addons.mozilla.org/services/redirect.php?url=http://www.google.com or another solution that causes the yellow bar to appear.

Comment 7

[Ryan Doherty (:rdoherty)]

Could we have the linking function not link to xpis?

I would hesitate creating a blind redirector, evil hackers could use it to mask the target url. 

Comment 8

[Justin Scott [:fligtar]]

(In reply to comment #7)
> Could we have the linking function not link to xpis?

No, the xpinstall dialog is based on content-type: application/x-xpinstall so there's no guarantee .xpi would be in the URL. For example: https://addons.mozilla.org/en-US/firefox/downloads/file/25412

Comment 9

[Ryan Doherty (:rdoherty)]

Wouldn't this be a bug with FF then? Using the page url instead of the xpi url to check against the whitelist?

Comment 10

[Justin Scott [:fligtar]]

That argument has been had many times - ask dveditz about it. I don't remember the reasoning, I've just come to accept it. Regardless of whether it's a Firefox bug or not, we have to deal with it.

Comment 11

[Ryan Doherty (:rdoherty)]

Can anyone think of any other options? I don't like the idea of a redirect script for 2 reasons. 

1) It masks the final url in the status bar
2) It can become a gateway for spammers to make links look valid

#1 could be fixed with JS (yuck). 
#2 could be fixed by checking the referer and if it's from AMO, forward it on.

Comment 12

[Wil Clouser [:clouserw]]

> #1 could be fixed with JS (yuck). 
Not awesome, but certainly alive and in widespread use out there on the internet.  It doesn't have accessibility problems and degrades gracefully.

> #2 could be fixed by checking the referer and if it's from AMO, forward it on.
This won't work for anyone that isn't sending referrers.  Not sure what the percentage is, but it's worth noting.

Thanks for bringing this up Justin - it's good to think about.

Comment 13

[Ryan Doherty (:rdoherty)]

#2 can be fixed with a hash for the url. Not a valid hash, no redirection. LinkedIn does it:

http://www.linkedin.com/redirect?url=http%3A%2F%2Fwww%2Eryandoherty%2Enet&urlhash=SJ8V

If that can be done the redirect solution seems good enough for me.

Comment 14

[Fred Wenzel [:wenzel]]

(In reply to comment #13)
> If that can be done the redirect solution seems good enough for me.

Yes, we just need to add some configurable string to the URL before hashing, because after all this is an open source app, so any hashing algorithm will be publicly visible.

Comment 16

[Morlan]

Hello, someone should fix the title. The word "automatically" is misspelled as "Automagically".

Comment 17

[Justin Scott [:fligtar]]

http://en.wiktionary.org/wiki/automagical

Comment 18

[Morlan]

For the record since this comment is relevant to this bug too:
https://bugzilla.mozilla.org/show_bug.cgi?id=507320#c8
(about comments and solution of xpi link issues)

Comment 19

[Wil Clouser [:clouserw]]

This is fixed in 5.3.