launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/text-area-widget-html-fix into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #746897 in Launchpad itself: "after ajax edit, bug description says "undefined""
  https://bugs.launchpad.net/launchpad/+bug/746897

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/text-area-widget-html-fix/+merge/55860

A recent change to plug some xss holes resulted in a bug which breaks xhr updates of text area fields after editing, eg bug description

== Implementation ==

Fix the creation of the node used to hold the html value extracted from the response packet.

== Tests ==

Update test_lp_client.js 
-- 
https://code.launchpad.net/~wallyworld/launchpad/text-area-widget-html-fix/+merge/55860
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/text-area-widget-html-fix into lp:launchpad.

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2011-03-31 06:31:37 +0000
+++ lib/lp/app/javascript/client.js	2011-04-01 02:05:50 +0000
@@ -460,7 +460,9 @@
       value = lp_html[key + '_link'];
     }
     if (value !== undefined) {
-      return Y.Node.create(value);
+      var result = Y.Node.create("<span/>");
+      result.setContent(value);
+      return result;
     }
   }
   return null;

=== modified file 'lib/lp/app/javascript/tests/test_lp_client.js'
--- lib/lp/app/javascript/tests/test_lp_client.js	2011-03-31 06:46:22 +0000
+++ lib/lp/app/javascript/tests/test_lp_client.js	2011-04-01 02:05:50 +0000
@@ -87,10 +87,10 @@
           'first': "Hello",
           'second': "World",
           'self_link': Y.lp.client.get_absolute_uri("a_self_link"),
-          'lp_html': {'first': "<p>Hello</p>"}
+          'lp_html': {'first': "<p>Hello</p><p>World</p>"}
         };
         var entry = new Y.lp.client.Entry(null, entry_repr, "a_self_link");
-        Assert.areEqual("Hello", entry.getHTML('first').get('innerHTML'));
+        Assert.areEqual("<p>Hello</p><p>World</p>", entry.getHTML('first').get('innerHTML'));
         // If there is no html representation, null is returned.
         Assert.areEqual(null, entry.getHTML('second'));
       },
@@ -152,7 +152,7 @@
         Assert.areEqual('first', first_event.name);
         Assert.areEqual('Hello', first_event.old_value);
         Assert.areEqual('World<boo/>', first_event.new_value);
-        Assert.areEqual('World html<boo></boo>', first_event.new_value_html.get('innerHTML'));
+        Assert.areEqual('<p>World html<boo></boo></p>', first_event.new_value_html.get('innerHTML'));
         Assert.areEqual(entry, first_event.entry);
 
         Assert.areEqual('second', second_event.name);

=== modified file 'lib/lp/blueprints/templates/specification-index.pt'
--- lib/lp/blueprints/templates/specification-index.pt	2011-03-31 04:52:03 +0000
+++ lib/lp/blueprints/templates/specification-index.pt	2011-04-01 02:05:50 +0000
@@ -330,7 +330,7 @@
             }
         });
         Y.on('lp:context:lifecycle_status:changed', function(e) {
-            Y.lp.ui.update_field('#lifecycle-status', e.new_value_html);
+            Y.lp.ui.update_field('#lifecycle-status', e.new_value);
         });
         Y.on('lp:context:is_started:changed', function(e) {
             var started = Y.one('#started-by');