Launchpad itself

Merge lp:~benji/launchpad/bug-777766 into lp:launchpad

  1. bug-777766
  2. Merge into devel
Proposed by Benji York
Status: Merged
Approved by: Benji York
Approved revision: no longer in the source branch.
Merged at revision: 12993
Proposed branch: lp:~benji/launchpad/bug-777766
Merge into: lp:launchpad
Diff against target: 44 lines (+13/-2)
2 files modified
lib/lp/app/widgets/popup.py (+9/-1)
lib/lp/app/widgets/tests/test_popup.py (+4/-1)
To merge this branch: bzr merge lp:~benji/launchpad/bug-777766
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+60100@code.launchpad.net

Commit message

[r=jcsackett][bug=777766] fix invalid ID generation

Description of the change

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.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
77
8__metaclass__ = type8__metaclass__ = type
99
10from base64 import urlsafe_b64encode
10import cgi11import cgi
1112
12import simplejson13import simplejson
@@ -94,7 +95,14 @@
9495
95 @property96 @property
96 def suffix(self):97 def suffix(self):
97 return self.name.replace('.', '-')98 """This is used to uniquify the widget ID to avoid ID collisions."""
99 # Since this will be used in an HTML ID, the allowable set of
100 # characters is smaller than the set that can appear in self.name.
101 # Therefore we use the URL-safe base 64 encoding of the name. However
102 # we also have to strip off any padding characters ("=") because
103 # Python's URL-safe base 64 encoding includes those and they aren't
104 # allowed in IDs either.
105 return urlsafe_b64encode(self.name).replace('=', '')
98106
99 @property107 @property
100 def show_widget_id(self):108 def show_widget_id(self):
101109
=== 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 @@
37 self.assertEqual(37 self.assertEqual(
38 simplejson.dumps(self.vocabulary.step_title),38 simplejson.dumps(self.vocabulary.step_title),
39 picker_widget.step_title_text)39 picker_widget.step_title_text)
40 # The widget name is encoded to get the widget's ID. The content of
41 # the ID is unimportant, the fact that it is unique on the page and a
42 # valid HTML element ID are what's important.
40 self.assertEqual(43 self.assertEqual(
41 'show-widget-field-teamowner', picker_widget.show_widget_id)44 'show-widget-ZmllbGQudGVhbW93bmVy', picker_widget.show_widget_id)
42 self.assertEqual(45 self.assertEqual(
43 'field.teamowner', picker_widget.input_id)46 'field.teamowner', picker_widget.input_id)
44 self.assertEqual(47 self.assertEqual(