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
1=== modified file 'lib/lp/app/widgets/popup.py'
2--- lib/lp/app/widgets/popup.py 2011-04-15 02:52:12 +0000
3+++ lib/lp/app/widgets/popup.py 2011-05-05 17:13:36 +0000
4@@ -7,6 +7,7 @@
5
6 __metaclass__ = type
7
8+from base64 import urlsafe_b64encode
9 import cgi
10
11 import simplejson
12@@ -94,7 +95,14 @@
13
14 @property
15 def suffix(self):
16- return self.name.replace('.', '-')
17+ """This is used to uniquify the widget ID to avoid ID collisions."""
18+ # Since this will be used in an HTML ID, the allowable set of
19+ # characters is smaller than the set that can appear in self.name.
20+ # Therefore we use the URL-safe base 64 encoding of the name. However
21+ # we also have to strip off any padding characters ("=") because
22+ # Python's URL-safe base 64 encoding includes those and they aren't
23+ # allowed in IDs either.
24+ return urlsafe_b64encode(self.name).replace('=', '')
25
26 @property
27 def show_widget_id(self):
28
29=== modified file 'lib/lp/app/widgets/tests/test_popup.py'
30--- lib/lp/app/widgets/tests/test_popup.py 2011-04-15 02:52:12 +0000
31+++ lib/lp/app/widgets/tests/test_popup.py 2011-05-05 17:13:36 +0000
32@@ -37,8 +37,11 @@
33 self.assertEqual(
34 simplejson.dumps(self.vocabulary.step_title),
35 picker_widget.step_title_text)
36+ # The widget name is encoded to get the widget's ID. The content of
37+ # the ID is unimportant, the fact that it is unique on the page and a
38+ # valid HTML element ID are what's important.
39 self.assertEqual(
40- 'show-widget-field-teamowner', picker_widget.show_widget_id)
41+ 'show-widget-ZmllbGQudGVhbW93bmVy', picker_widget.show_widget_id)
42 self.assertEqual(
43 'field.teamowner', picker_widget.input_id)
44 self.assertEqual(