Merge lp:~deryck/launchpad/better-html-handling-description-editing into lp:launchpad

[Deryck Hodge]

Do not use formatter config option to handle html in TextAreaEditor Widget. JS API client can do a patch on individual fields now.

[Deryck Hodge (deryck)]

= Summary =

Tim had some troubles getting an xhtml representation of an object
displaying properly with the ajax multi-line editor used in bug
description editing. A launchpad-dev mailing list discussion around
this uncovered that the API will allow us to patch a field on an object
and get back just the field's representation, rather than patching an
object and getting back a representation of the object.

Bug #460725 records the need to update the widget used in multi-line
editing to not rely on formatter config option hacks to parse the object
xhtml. This means the widget should just patch the field.

== Proposed fix ==

This branch updates TextAreaEditorWidget to remove the formatter option.
 client.js also had to be patched to allow for submitting a single field
object patch request.

== Pre-implementation notes ==

There was discussion on the launchpad-dev mailing list with Francis that
led to private discussions with Leonard about how to correctly do this.
 He's pretty much looked at this diff in stages by this point.

== Implementation details ==

While the API is capable of doing this single field patch request, the
JavaScript client was not enabled for it and had to be updated.
Otherwise, it's a pretty simple branch removing the formatter option and
properly encoding the response.

(The first few lines of the diff are cleanup from lint warnings
unrelated to this change.)

There is a related lazr-js branch that must land when this one does.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/widgets/lazrjs.py
  lib/canonical/launchpad/javascript/client/client.js
  lib/lp/bugs/browser/bug.py

== JSLint notices ==
jslint: No problem found in
'/home/deryck/canonical/lp-branches/better-html-handling-description-editing/lib/canonical/launchpad/javascript/client/client.js'.

jslint: 1 file to lint.

== Pylint notices ==

lib/canonical/widgets/lazrjs.py
    23: [F0401] Unable to import 'lazr.restful.interfaces' (No module
named restful)

lib/lp/bugs/browser/bug.py
    44: [F0401] Unable to import 'lazr.enum' (No module named enum)
    45: [F0401] Unable to import 'lazr.lifecycle.event' (No module named
lifecycle)
    46: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module
named lifecycle)
    47: [F0401] Unable to import 'lazr.restful.interfaces' (No module
named restful)

[Leonard Richardson (leonardr)]

review: Approve

=== modified file 'lib/canonical/launchpad/javascript/client/client.js'
--- lib/canonical/launchpad/javascript/client/client.js	2009-09-17 16:27:19 +0000
+++ lib/canonical/launchpad/javascript/client/client.js	2009-10-27 17:44:13 +0000
@@ -43,20 +43,24 @@
         var keys = [];
         var key;
         for (key in collection) {
-            keys.push(key);
+            if (collection.hasOwnProperty(key)) {
+                keys.push(key);
+            }
         }
         keys.sort();
         for (var index in keys) {
-            key = keys[index];
-            var value;
-            try {
-                value = format(collection[key]);
-            } catch (e) {
-                // This is necessary to handle attributes which
-                // will throw a permission denied error.
-                value = e.message;
+            if (keys.hasOwnProperty(index)) {
+                key = keys[index];
+                var value;
+                try {
+                    value = format(collection[key]);
+                } catch (e) {
+                    // This is necessary to handle attributes which
+                    // will throw a permission denied error.
+                    value = e.message;
+                }
+                items.push(key + '=' + value);
             }
-            items.push(key + '=' + value);
         }
         return items.join(',\n  ');
     };
@@ -574,6 +578,15 @@
         resource: {},
 
         /**
+         * Is this a patch for only a field,
+         * not the entire resource object?
+         *
+         * @attribute patch_field
+         * @type Boolean
+         */
+        patch_field: false,
+
+        /**
          * The function to use to format the returned result into a form that
          * can be inserted into the page DOM.
          *
@@ -727,9 +740,16 @@
 
         var client =  new LP.client.Launchpad();
         var formatter = Y.bind(this.get('formatter'), this);
-        var patch_payload = {};
         var attribute = this.get('patch');
-        patch_payload[attribute] = owner.getInput();
+
+        var patch_payload;
+        var val = owner.getInput();
+        if (this.get('patch_field')) {
+            patch_payload = val;
+        } else {
+            patch_payload = {};
+            patch_payload[attribute] = val;
+        }
 
         var callbacks = {
             on: {
@@ -766,7 +786,11 @@
      * the DOM.
      */
     _defaultFormatter: function(result, attribute) {
-        return result.get(attribute);
+        if (Y.Lang.isString(result)) {
+            return result;
+        } else {
+            return result.get(attribute);
+        }
     }
 });
 

=== modified file 'lib/canonical/widgets/lazrjs.py'
--- lib/canonical/widgets/lazrjs.py	2009-09-17 19:09:29 +0000
+++ lib/canonical/widgets/lazrjs.py	2009-10-27 17:44:13 +0000
@@ -207,29 +207,9 @@
             widget.editor.plug({
                 fn: Y.lp.client.plugins.PATCHPlugin, cfg: {
                   patch: '%(attribute)s',
-                  resource: '%(context_url)s',
-                  accept: 'application/xhtml+xml',
-                  formatter: function(result, attribute) {
-                      var dl = Y.DOM.create(result)[1]
-                      var dl_nodes = dl.childNodes;
-                      var i;
-                      // <dd> in XHR responses breaks description formatting.
-                      for (i=0; i<dl_nodes.length; i++) {
-                          var child = dl_nodes[i];
-                          // Ignore text nodes when looking for the attribute.
-                          // 3 is the text nodeType.
-                          if (child.nodeType != 3 &&
-                              child.firstChild.textContent == attribute) {
-                              var ptags = dl_nodes[i+2].childNodes;
-                              var span = Y.Node.create('<span></span>');
-                              var n;
-                              for (n=0; n<ptags.length; n++) {
-                                  span.appendChild(ptags[n]);
-                              }
-                              return span;
-                          }
-                      }
-                  }
+                  resource: '%(context_url)s/%(attribute)s',
+                  patch_field: true,
+                  accept: 'application/xhtml+xml'
             }});
             if (!Y.UA.opera) {
                 widget.render();

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2009-10-23 18:07:36 +0000
+++ lib/lp/bugs/browser/bug.py	2009-10-27 17:44:13 +0000
@@ -933,5 +933,5 @@
     def renderer(value):
         nomail  = formatter(value).obfuscate_email()
         html    = formatter(nomail).text_to_html()
-        return html
+        return html.encode('utf-8')
     return renderer