launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-957524 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #957524 in Launchpad itself: "+sharing breaks when a person name contains special characters"
  https://bugs.launchpad.net/launchpad/+bug/957524
  Bug #957545 in Launchpad itself: "Modification operations on +sharing break in IE8"
  https://bugs.launchpad.net/launchpad/+bug/957545

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-957524/+merge/98030

Selectors like #someid don't work when someid contains special characters like ".". Person names can contain such special characters, so the sharee table rendering was breaking when some people were shown. I fixed the JS to use [id=someid] instead of #someid, and updated the tests to demonstrate that this works.

I also fixed +sharing in IE8. pillarsharingview.js was using :nth-child (unsupported by IE8) without declaring a dependency on selector-css3.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-957524/+merge/98030
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-957524 into lp:launchpad.

=== modified file 'lib/lp/registry/javascript/sharing/pillarsharingview.js'
--- lib/lp/registry/javascript/sharing/pillarsharingview.js	2012-03-16 08:18:37 +0000
+++ lib/lp/registry/javascript/sharing/pillarsharingview.js	2012-03-16 23:50:26 +0000
@@ -380,8 +380,8 @@
 namespace.PillarSharingView = PillarSharingView;
 
 }, "0.1", { "requires": [
-    'node', 'lp.client', 'lp.mustache', 'lazr.picker', 'lp.app.picker',
-    'lp.mustache', 'lp.registry.sharing.shareepicker',
+    'node', 'selector-css3', 'lp.client', 'lp.mustache', 'lazr.picker',
+    'lp.app.picker', 'lp.mustache', 'lp.registry.sharing.shareepicker',
     'lp.registry.sharing.shareetable', 'lp.app.confirmationoverlay'
     ]});
 

=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js	2012-03-16 08:18:37 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js	2012-03-16 23:50:26 +0000
@@ -179,9 +179,10 @@
         });
 
         var id = 'permission-'+sharee.name;
-        var sharee_row = this.get('sharee_table').one('#' + id);
-        var permission_node = sharee_row.one('#td-' + id);
-        var contentBox = permission_node.one('#' + policy + '-' + id);
+        var sharee_row = this.get('sharee_table').one('[id=' + id + ']');
+        var permission_node = sharee_row.one('[id=td-' + id + ']');
+        var contentBox = permission_node.one(
+            '[id=' + policy + '-' + id + ']');
         var value_location = contentBox.one('.value');
         var editicon = permission_node.one('a.editicon');
 

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js	2012-03-16 01:42:53 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js	2012-03-16 23:50:26 +0000
@@ -19,7 +19,7 @@
                      'role': '(Maintainer)', web_link: '~fred',
                      'self_link': '~fred',
                      'permissions': {'P1': 's1', 'P2': 's2'}},
-                    {'name': 'john', 'display_name': 'John Smith',
+                    {'name': 'john.smith', 'display_name': 'John Smith',
                      'role': '', web_link: '~smith', 'self_link': '~smith',
                     'permissions': {'P1': 's1', 'P3': 's3'}}
                     ],