Merge lp:~jcsackett/launchpad/linkifier-bugs into lp:launchpad

[j.c.sackett]

[r=mars][ui=none][bug=85470,179868,276726] Updates the protocols recognized by the linkifier. file:// no longer supported, apt://, apt: and data: added.

Summary
=======

Several simple bugs have been filed against the linkifier in bug comments regarding its handling of various protocols. These are handled in this branch by simple changes to the protocol section of the regex used in the linkifier.

Preimp-talk
===========

Spoke with Curtis Hovey.

Implementation
==============

The linkifier decides which things to link based on the protocols defined in the protocol glob of a truly massive regex in lp.app.browser.stringformatter

The protocol section is modified to support apt and data uris, and to not follow file uris, as the latter is useless to anyone but the person with the file on their computer.

Tests
=====

lp test -t test_stringformatter

QA
==

Open launchpad.dev and file a bug against any product. In the comment section, put the following:

This links: apt:some-package
As does this: apt://some-package
As does this: data:text/plain,hello
This does not: file://some/file.txt

[Māris Fogels (mars)]

Hi J.C.,

An excellent change, r=mars

I am surprised you have to use the DatabaseFunctionalLayer. The formatter objects must be doing a lot of work? Could we get away with just Zope registry, or even less, in this test?

Maris

review: Approve

[j.c.sackett (jcsackett)]

On Dec 14, 2010, at 2:21 PM, Māris Fogels wrote:
> I am surprised you have to use the DatabaseFunctionalLayer. The formatter objects must be doing a lot of work? Could we get away with just Zope registry, or even less, in this test?

Actually, no layer is needed. I am just incredibly used to typing layer=DatabaseFunctionalLayer.

I've whacked the layer and everything is still dandy. Thanks for catching that.

//j.c.

=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py	2010-10-22 03:35:41 +0000
+++ lib/lp/app/browser/stringformatter.py	2010-12-14 20:00:39 +0000
@@ -360,7 +360,7 @@
     _re_linkify = re.compile(r'''
       (?P<url>
         \b
-        (?:about|gopher|http|https|sftp|news|ftp|mailto|file|irc|jabber)
+        (?:about|gopher|http|https|sftp|news|ftp|mailto|irc|jabber|apt|data)
         :
         (?:
           (?:

=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py	2010-10-04 19:50:45 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py	2010-12-14 20:00:39 +0000
@@ -107,9 +107,46 @@
       <tag>1234567890123456</tag>
     """
 
+class TestLinkifyingProtocols(TestCase):
+
+    def test_apt_is_linked(self):
+        test_string = 'This becomes a link: apt:some-package'
+        html = FormattersAPI(test_string).text_to_html()
+        expected_html = (
+            '<p>This becomes a link: '
+            '<a rel="nofollow" '
+                'href="apt:some-package">apt:some-<wbr></wbr>package</a></p>')
+        self.assertEqual(expected_html, html)
+
+        # Do it again for apt://
+        test_string = 'This becomes a link: apt://some-package'
+        html = FormattersAPI(test_string).text_to_html()
+        expected_html = (
+            '<p>This becomes a link: '
+            '<a rel="nofollow" '
+                'href="apt://some-package">apt://some-<wbr></wbr>package</a></p>')
+        self.assertEqual(expected_html, html)
+
+    def test_file_is_not_linked(self):
+        test_string = "This doesn't become a link: file://some/file.txt"
+        html = FormattersAPI(test_string).text_to_html()
+        expected_html = (
+            "<p>This doesn't become a link: "
+            "file://<wbr></wbr>some/file.<wbr></wbr>txt</p>")
+        self.assertEqual(expected_html, html)
+
+    def test_data_is_linked(self):
+        test_string = "This becomes a link: data:text/plain,test"
+        html = FormattersAPI(test_string).text_to_html()
+        expected_html = (
+            "<p>This becomes a link: "
+            '<a rel="nofollow" '
+            'href="data:text/plain,test">data:text/<wbr></wbr>plain,test</a></p>')
+        self.assertEqual(expected_html, html)
+            
 
 class TestDiffFormatter(TestCase):
-    """Test the string formtter fmt:diff."""
+    """Test the string formatter fmt:diff."""
     layer = DatabaseFunctionalLayer
 
     def test_emptyString(self):