launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/linkifier-bugs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #85470 linkification should not include file://
  https://bugs.launchpad.net/bugs/85470
  #179868 apt:package-name URLs aren't auto-linkified
  https://bugs.launchpad.net/bugs/179868
  #276726 URL auto-linking doesn't linkify data: URLs
  https://bugs.launchpad.net/bugs/276726


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
-- 
https://code.launchpad.net/~jcsackett/launchpad/linkifier-bugs/+merge/43677
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/linkifier-bugs into lp:launchpad.

=== 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 17:49:57 +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 17:49:57 +0000
@@ -107,9 +107,48 @@
       <tag>1234567890123456</tag>
     """
 
+class TestLinkifyingProtocols(TestCase):
+    
+    layer = DatabaseFunctionalLayer
+
+    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):