launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-777766 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-777766/+merge/60100

Bug 777766 describes a JS error that occurs because an element ID
contains invalid characters.  This branch (url-safe) base 64 encodes the
name before using it in the ID.

The make lint report is clean.

The tests for the widget can be run thusly:

    bin/test -c -t test_popup -m lp.app.widgets.tests

To QA this branch create a project with a plus in it's URL, create a bug
in that project and then navigate to the bug page.  If no JS errors are
reported and the Subscribers portlet is populated, it worked.

-- 
https://code.launchpad.net/~benji/launchpad/bug-777766/+merge/60100
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-777766 into lp:launchpad.

=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py	2011-04-15 02:52:12 +0000
+++ lib/lp/app/widgets/popup.py	2011-05-05 17:13:36 +0000
@@ -7,6 +7,7 @@
 
 __metaclass__ = type
 
+from base64 import urlsafe_b64encode
 import cgi
 
 import simplejson
@@ -94,7 +95,14 @@
 
     @property
     def suffix(self):
-        return self.name.replace('.', '-')
+        """This is used to uniquify the widget ID to avoid ID collisions."""
+        # Since this will be used in an HTML ID, the allowable set of
+        # characters is smaller than the set that can appear in self.name.
+        # Therefore we use the URL-safe base 64 encoding of the name.  However
+        # we also have to strip off any padding characters ("=") because
+        # Python's URL-safe base 64 encoding includes those and they aren't
+        # allowed in IDs either.
+        return urlsafe_b64encode(self.name).replace('=', '')
 
     @property
     def show_widget_id(self):

=== modified file 'lib/lp/app/widgets/tests/test_popup.py'
--- lib/lp/app/widgets/tests/test_popup.py	2011-04-15 02:52:12 +0000
+++ lib/lp/app/widgets/tests/test_popup.py	2011-05-05 17:13:36 +0000
@@ -37,8 +37,11 @@
         self.assertEqual(
             simplejson.dumps(self.vocabulary.step_title),
             picker_widget.step_title_text)
+        # The widget name is encoded to get the widget's ID.  The content of
+        # the ID is unimportant, the fact that it is unique on the page and a
+        # valid HTML element ID are what's important.
         self.assertEqual(
-            'show-widget-field-teamowner', picker_widget.show_widget_id)
+            'show-widget-ZmllbGQudGVhbW93bmVy', picker_widget.show_widget_id)
         self.assertEqual(
             'field.teamowner', picker_widget.input_id)
         self.assertEqual(