launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/fix_attr_quotes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/fix_attr_quotes/+merge/113610

= Summary =

In YUI 3.5 all selectors based on attribute values must be quoted. This is a
clean up based on the search through the codebase for selectors missing the
quotes.

== Implementation Notes ==

The search used was:

grep -P "\([\"']\w+\[[^'\"]*\]" lib/lp/**/*.(pt|js)

This only changes quotes and no other changes to the files/setup.

== Tests ==

./bin/test -x -cvv --layer=YUITestLayer


== LoC Qualification ==

This is an even LoC change. We're only changing quotes not adding/removing any
maint.
-- 
https://code.launchpad.net/~rharding/launchpad/fix_attr_quotes/+merge/113610
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/fix_attr_quotes into lp:launchpad.

=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js'
--- lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js	2012-06-15 04:07:30 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js	2012-07-05 16:50:24 +0000
@@ -394,7 +394,7 @@
                 .set("choices", ["a", "b"]);
             ArrayAssert.itemsAreSame(
                 [], this.widget.get("choice"));
-            this.widget.fieldNode.one("input[value=a]")
+            this.widget.fieldNode.one("input[value='a']")
                 .set("checked", "checked");
             ArrayAssert.itemsAreSame(
                 ["a"], attrselect("value")(this.widget.get("choice")));
@@ -407,12 +407,12 @@
                 .set("multiple", false)
                 .set("choices", ["a", "b"]);
             Assert.isNull(this.widget.get("choice"));
-            this.widget.fieldNode.one("input[value=a]")
+            this.widget.fieldNode.one("input[value='a']")
                 .set("checked", "checked");
             Assert.areSame("a", this.widget.get("choice").value);
-            this.widget.fieldNode.one("input[value=b]")
+            this.widget.fieldNode.one("input[value='b']")
                 .set("checked", "checked");
-            if (this.widget.fieldNode.one("input[value=a]").get("checked")) {
+            if (this.widget.fieldNode.one("input[value='a']").get("checked")) {
                 // This assertion can only be made if the DOM/JS is broken
                 // in the host browser.
                 Assert.isUndefined(this.widget.get("choice"));

=== modified file 'lib/lp/app/widgets/templates/license.pt'
--- lib/lp/app/widgets/templates/license.pt	2012-05-24 21:38:54 +0000
+++ lib/lp/app/widgets/templates/license.pt	2012-07-05 16:50:24 +0000
@@ -131,8 +131,8 @@
 
         // When Other/Proprietary or Other/Open Source is chosen, the
         // license_info widget is displayed.
-        var other_com = Y.one('input[value=OTHER_PROPRIETARY]');
-        var other_os = Y.one('input[value=OTHER_OPEN_SOURCE]');
+        var other_com = Y.one('input[value="OTHER_PROPRIETARY"]');
+        var other_os = Y.one('input[value="OTHER_OPEN_SOURCE"]');
         var details = Y.one('#license-details');
         var proprietary = Y.one('#proprietary');
 

=== modified file 'lib/lp/bugs/javascript/official_bug_tags.js'
--- lib/lp/bugs/javascript/official_bug_tags.js	2012-04-26 19:11:37 +0000
+++ lib/lp/bugs/javascript/official_bug_tags.js	2012-07-05 16:50:24 +0000
@@ -429,7 +429,7 @@
     });
 
     // All went well, hide the plain html form.
-    Y.one('form[name=launchpadform]').setStyle('display', 'none');
+    Y.one('form[name="launchpadform"]').setStyle('display', 'none');
 };
 }, "0.1", {
     "requires": [

=== modified file 'lib/lp/code/javascript/sourcepackagerecipe.new.js'
--- lib/lp/code/javascript/sourcepackagerecipe.new.js	2010-12-13 08:23:15 +0000
+++ lib/lp/code/javascript/sourcepackagerecipe.new.js	2012-07-05 16:50:24 +0000
@@ -30,7 +30,7 @@
     }
 
     module.onclick_use_ppa = function(e) {
-      var value = getRadioSelectedValue('input[name=field.use_ppa]');
+      var value = getRadioSelectedValue('input[name="field.use_ppa"]');
       if (value == 'create-new') {
         set_enabled(PPA_NAME_ID, true);
         set_enabled(PPA_SELECTOR_ID, false);
@@ -42,7 +42,7 @@
     };
 
     module.setup = function() {
-       Y.all('input[name=field.use_ppa]').on(
+       Y.all('input[name="field.use_ppa"]').on(
           'click', module.onclick_use_ppa);
 
        // Set the initial state.

=== modified file 'lib/lp/registry/javascript/sharing/shareepicker.js'
--- lib/lp/registry/javascript/sharing/shareepicker.js	2012-06-22 03:25:33 +0000
+++ lib/lp/registry/javascript/sharing/shareepicker.js	2012-07-05 16:50:24 +0000
@@ -122,7 +122,7 @@
             sharing_permissions, disabled_some_types);
         step_two_content.one('div.step-links')
             .insert(policy_selector, 'before');
-        step_two_content.all('input[name^=field.permission]')
+        step_two_content.all('input[name^="field.permission"]')
                 .on('click', function(e) {
             self._disable_select_if_all_info_choices_nothing(step_two_content);
         });
@@ -162,7 +162,7 @@
             }
         }, '.next', this);
         // Initially set all radio buttons to Nothing.
-        step_two_content.all('input[name^=field.permission][value=NOTHING]')
+        step_two_content.all('input[name^="field.permission"][value="NOTHING"]')
                 .each(function(radio_button) {
             radio_button.set('checked', true);
         });
@@ -190,7 +190,7 @@
      */
     _all_info_choices_nothing: function(content) {
         var all_unticked = true;
-        content.all('input[name^=field.permission]')
+        content.all('input[name^="field.permission"]')
                 .each(function(info_node) {
             if (info_node.get('value') !== 'NOTHING') {
                 all_unticked &= !info_node.get('checked');

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareepicker.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareepicker.js	2012-06-22 03:25:33 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareepicker.js	2012-07-05 16:50:24 +0000
@@ -128,8 +128,8 @@
             Y.Array.each(this.information_types, function(info_type) {
                 Y.Array.each(this.sharing_permissions, function(permission) {
                     var rb = step_two_content.one(
-                        'input[name=field.permission.' + info_type.value +
-                        '][value="' + permission.value + '"]');
+                        'input[name="field.permission.' + info_type.value +
+                        '"][value="' + permission.value + '"]');
                     Y.Assert.isNotNull(rb);
                 });
             });
@@ -187,11 +187,11 @@
                 'Update sharing policies for Fred', steptitle);
             // By default, selections only for ALL and NOTHING are available
             // (and no others).
-            Y.Assert.isNotNull(cb.one('input[value=ALL]'));
-            Y.Assert.isNotNull(cb.one('input[value=NOTHING]'));
-            Y.Assert.isNull(cb.one('input[value=SOME]'));
+            Y.Assert.isNotNull(cb.one('input[value="ALL"]'));
+            Y.Assert.isNotNull(cb.one('input[value="NOTHING"]'));
+            Y.Assert.isNull(cb.one('input[value="SOME"]'));
             // Selected permission checkboxes should be ticked.
-            cb.all('input[name=field.permission.P1]')
+            cb.all('input[name="field.permission.P1"]')
                     .each(function(node) {
                 if (node.get('checked')) {
                     Y.Assert.areEqual('ALL', node.get('value'));
@@ -200,7 +200,7 @@
                 }
             });
             Y.Array.each(['P2', 'P3'], function(info_type) {
-                cb.all('input[name=field.permission.' + info_type + ']')
+                cb.all('input[name="field.permission.' + info_type + '"]')
                         .each(function(node) {
                     if (node.get('checked')) {
                         Y.Assert.areEqual('NOTHING', node.get('value'));
@@ -232,9 +232,9 @@
                 allowed_permissions: ['SOME']
             });
             var cb = this.picker.get('contentBox');
-            Y.Assert.isNull(cb.one('input[value=ALL]'));
-            Y.Assert.isNull(cb.one('input[value=NOTHING]'));
-            Y.Assert.isNotNull(cb.one('input[value=SOME]'));
+            Y.Assert.isNull(cb.one('input[value="ALL"]'));
+            Y.Assert.isNull(cb.one('input[value="NOTHING"]'));
+            Y.Assert.isNotNull(cb.one('input[value="SOME"]'));
         },
 
         // Test that selected radio buttons for permission type 'Some' can be
@@ -261,7 +261,7 @@
                 Y.Assert.areEqual(expected_disabled, node.get('disabled'));
             };
             var cb = this.picker.get('contentBox');
-            cb.all('input[type=radio][name^=field.permission]').each(
+            cb.all('input[type="radio"][name^="field.permission"]').each(
                 check_permission_node);
         },
 
@@ -314,7 +314,7 @@
             var step_two_content = cb.one('.picker-content-two');
             // Select an access policy.
             step_two_content
-                .one('input[name=field.permission.P2][value="ALL"]')
+                .one('input[name="field.permission.P2"][value="ALL"]')
                 .simulate('click');
             var select_link = step_two_content.one('a.next');
             select_link.simulate('click');
@@ -354,7 +354,7 @@
                 '.yui3-picker-results li:nth-child(2)').simulate('click');
             // Select an access policy.
             step_two_content
-                .one('input[name=field.permission.P2][value="ALL"]')
+                .one('input[name="field.permission.P2"][value="ALL"]')
                 .simulate('click');
             var select_link = step_two_content.one('a.next');
             select_link.simulate('click');
@@ -398,13 +398,13 @@
             select_link.simulate('click');
             Y.Assert.isFalse(save_called);
             // Select one info type and check submit links are enabled.
-            cb.one('input[name=field.permission.P1][value=ALL]')
+            cb.one('input[name="field.permission.P1"][value="ALL"]')
                 .simulate('click');
             cb.all('a.next', function(link) {
                 Y.Assert.isFalse(link.hasClass('invalid-link'));
             });
             // De-select info type and submit links are disabled again.
-            cb.one('input[name=field.permission.P1][value=NOTHING]')
+            cb.one('input[name="field.permission.P1"][value="NOTHING"]')
                 .simulate('click');
             cb.all('a.next', function(link) {
                 Y.Assert.isTrue(link.hasClass('invalid-link'));
@@ -412,7 +412,7 @@
             select_link.simulate('click');
             Y.Assert.isFalse(save_called);
             // Once at least one info type is selected, save is called.
-            cb.one('input[name=field.permission.P1][value=ALL]')
+            cb.one('input[name="field.permission.P1"][value="ALL"]')
                 .simulate('click');
             select_link.simulate('click');
             Y.Assert.isTrue(save_called);

=== modified file 'lib/lp/registry/javascript/tests/test_team.js'
--- lib/lp/registry/javascript/tests/test_team.js	2012-04-06 17:28:25 +0000
+++ lib/lp/registry/javascript/tests/test_team.js	2012-07-05 16:50:24 +0000
@@ -63,7 +63,7 @@
         test_visibility_change_private: function() {
             namespace.visibility_changed('PRIVATE');
             var nr_radio_buttons = 0;
-            Y.all('input[type=radio]').each(function(radio_button) {
+            Y.all('input[type="radio"]').each(function(radio_button) {
                 if (radio_button.ancestor('tr').hasClass('unseen')
                         || !radio_button.get('checked')) {
                     return;
@@ -103,7 +103,7 @@
 
             namespace.visibility_changed('PUBLIC');
             var nr_radio_buttons = 0;
-            Y.all('input[type=radio]').each(function(radio_button) {
+            Y.all('input[type="radio"]').each(function(radio_button) {
                 Y.Assert.isFalse(
                     radio_button.ancestor('tr').hasClass('unseen'));
                 var help_row = radio_button.ancestor('tr')

=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt	2012-03-02 16:17:46 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt	2012-07-05 16:50:24 +0000
@@ -65,7 +65,7 @@
             'lp.app.confirmationoverlay',function(Y) {
             Y.on('domready', function() {
               var dsd_details = Y.lp.registry.distroseriesdifferences_details;
-              Y.all('input[name=field.actions.sync]').each(function(button) {
+              Y.all('input[name="field.actions.sync"]').each(function(button) {
                 // Cleanup the button's title which says the button is disabled if
                 // Javascript is disabled.
                 button.set('title', '');