launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/ie-fixes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #337397 in Launchpad itself: "Inline editor is broken in IE7"
  https://bugs.launchpad.net/launchpad/+bug/337397

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/ie-fixes/+merge/98555

Unbreak and undisable various bits of JS in IE>=8. This brings them almost up to full support.

All pretty trivially inoffensive changes, except for the structural subscriptions HTML thingy. That one is just a rewording to be prettier and not be invalid (note the typo in the original which meant that instead of generating a <td><label>...</label></td>, it generated a <td></td><td></td><label>...</label>\).
-- 
https://code.launchpad.net/~wgrant/launchpad/ie-fixes/+merge/98555
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/ie-fixes into lp:launchpad.

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2012-01-10 14:24:19 +0000
+++ lib/lp/app/javascript/client.js	2012-03-21 02:22:21 +0000
@@ -345,7 +345,7 @@
             // notifications.
             var notifications = response.getResponseHeader(
                 'X-Lazr-Notifications');
-            if (notifications !== null) {
+            if (notifications !== null && notifications !== "") {
                 module.display_notifications(notifications);
             }
             if (Y.Lang.isValue(representation) &&
@@ -391,7 +391,8 @@
     if (notifications === undefined) {
         return;
     }
-    if (notifications === 'null' || notifications === null) {
+    if (notifications === 'null' || notifications === null
+        || notifications === "") {
         module.remove_notifications();
         return;
     }

=== modified file 'lib/lp/app/javascript/inlineedit/editor.js'
--- lib/lp/app/javascript/inlineedit/editor.js	2012-03-06 18:13:52 +0000
+++ lib/lp/app/javascript/inlineedit/editor.js	2012-03-21 02:22:21 +0000
@@ -1199,13 +1199,6 @@
             // effectively shuts down the widget.  See bug #331584.
             e.preventDefault();
         }
-
-        // XXX JeroenVermeulen 2009-03-11 bug=341098: The editor breaks
-        // in IE.  Fix the bug, or at least narrow this down to fewer IE
-        // versions.
-        if (Y.UA.ie) {
-            e.preventDefault();
-        }
     },
 
     /**
@@ -1376,9 +1369,6 @@
      * @method renderer
      */
     renderer: function() {
-        if (this.editor.get(MULTILINE) && (Y.UA.ie || Y.UA.opera)) {
-            return;
-        }
         EditableText.superclass.renderer.apply(this, arguments);
     }
 });

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2012-02-22 05:31:47 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2012-03-21 02:22:21 +0000
@@ -56,9 +56,6 @@
  * @param show_widget_id the id of the Choose... link
  */
 namespace.addPicker = function(config, show_widget_id) {
-    if (Y.UA.ie) {
-        return;
-    }
     _addPicker(config, show_widget_id);
     Y.on('lp.widget.connect', function(e) {
         _addPicker(config, show_widget_id);
@@ -99,10 +96,6 @@
     vocabulary, resource_uri, attribute_name,
     content_box_id, config) {
 
-    if (Y.UA.ie) {
-        return;
-    }
-
     var null_display_value = 'None';
     var show_search_box = true;
     var vocabulary_filters;
@@ -411,10 +404,6 @@
   */
 namespace.create = function (vocabulary, config, associated_field_id,
                              vocabulary_filters) {
-    if (Y.UA.ie) {
-        return;
-    }
-
     var header = 'Choose an item.';
     var step_title = "Enter search terms";
     var picker_type = "default";
@@ -494,7 +483,7 @@
         var validation_callbacks = [];
         var config_validate_callbacks = config.validate_callback;
         if (Y.Lang.isArray(config_validate_callbacks)) {
-            config_validate_callbacks.forEach(function(callback) {
+            Y.Array.forEach(config_validate_callbacks, function(callback) {
                 validation_callbacks.push(callback);
             });
         } else if (Y.Lang.isFunction(config_validate_callbacks)) {
@@ -503,7 +492,7 @@
         var val_ns = Y.namespace('lp.app.picker.validation');
         var other_validate_callbacks = val_ns[config.show_widget_id];
         if (Y.Lang.isArray(other_validate_callbacks)) {
-            other_validate_callbacks.forEach(function(callback) {
+            Y.Array.forEach(other_validate_callbacks, function(callback) {
                 validation_callbacks.push(callback);
             });
         } else if (Y.Lang.isFunction(other_validate_callbacks)) {

=== modified file 'lib/lp/app/templates/inline-picker.pt'
--- lib/lp/app/templates/inline-picker.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/inline-picker.pt	2012-03-21 02:22:21 +0000
@@ -30,10 +30,6 @@
 <script tal:condition="view/can_write"
         tal:content="structure string:
 LPJS.use('lp.app.picker', 'lp.client', function(Y) {
-    if (Y.UA.ie) {
-        return;
-    }
-
     var picker_config = ${view/json_config}
     picker_config.validate_callback = Y.lp.app.picker.public_private_warning;
     Y.on('load', function(e) {

=== modified file 'lib/lp/app/templates/text-area-editor.pt'
--- lib/lp/app/templates/text-area-editor.pt	2012-03-05 09:17:17 +0000
+++ lib/lp/app/templates/text-area-editor.pt	2012-03-21 02:22:21 +0000
@@ -29,9 +29,7 @@
                   resource: ${view/json_resource_uri},
                   use_html: true
             }});
-            if (!Y.UA.opera) {
-                widget.render();
-            }
+            widget.render();
             var lpns = Y.namespace('lp');
             if (!lpns.widgets) {
                 lpns.widgets = {};

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2012-02-01 23:40:45 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2012-03-21 02:22:21 +0000
@@ -47,6 +47,10 @@
      * for these forms.
      */
     Y.on('load', function() {
+        /* Skip all this in IE for now. The dupe form's textbox is shown
+         * even when the form is hidden, and there's possibly other
+         * issues.
+         */
         if (Y.UA.ie) {
             return;
         }
@@ -885,6 +889,9 @@
  * @method setup_bugtasks_row
  */
 namespace.setup_bugtask_row = function(conf) {
+    /* ChoiceSource doesn't seem to function in IE at present, breaking
+     * all of this AJAX except the assignee and product pickers.
+     */
     if (Y.UA.ie) {
         return;
     }
@@ -1189,7 +1196,7 @@
  * @method setup_me_too
  */
 namespace.setup_me_too = function(user_is_affected, others_affected_count) {
-    // IE (7 & 8 tested) is stupid, stupid, stupid.
+    /* ChoiceSource is broken in IE8, probably IE9. */
     if (Y.UA.ie) {
         return;
     }

=== modified file 'lib/lp/registry/javascript/milestoneoverlay.js'
--- lib/lp/registry/javascript/milestoneoverlay.js	2012-01-11 10:39:27 +0000
+++ lib/lp/registry/javascript/milestoneoverlay.js	2012-03-21 02:22:21 +0000
@@ -191,6 +191,9 @@
       *                            the milestone is created.
       */
     module.attach_widget = function(config) {
+        /* In IE8 the text widgets stay around after the form is closed,
+         * so disable there for now.
+         */
         if (Y.UA.ie) {
             return;
         }

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2012-03-14 12:53:47 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2012-03-21 02:22:21 +0000
@@ -441,13 +441,14 @@
  * @param {String} name Name of the control.
  */
 function make_cell(item, name) {
-    var cell = Y.Node.create('<td style="padding-left:3px"><td>');
-    cell.appendChild('<label></label>')
-        .append(Y.Node.create('<input type="checkbox" checked></input>')
-            .set('name', name)
-            .set('value', item))
-        .append(Y.Node.create('<span></span>')
-            .set('text', item));
+    var cell = Y.Node.create(
+        '<td style="padding-left:3px"><label>' +
+        '<input type="checkbox" checked="checked" />' +
+        '<span></></label></td>');
+    cell.one('span').set('text', item);
+    cell.one('input')
+        .set('name', name)
+        .set('value', item);
     return cell;
 }
 /**

=== modified file 'lib/lp/registry/javascript/team.js'
--- lib/lp/registry/javascript/team.js	2012-03-05 15:34:57 +0000
+++ lib/lp/registry/javascript/team.js	2012-03-21 02:22:21 +0000
@@ -15,10 +15,6 @@
  * @method setup_add_member_handler
  */
 module.setup_add_member_handler = function(step_title) {
-    if (Y.UA.ie) {
-        return;
-    }
-
     var config = {
         header: 'Add a member',
         step_title: step_title,

=== modified file 'lib/lp/services/doc/sprites.txt'
--- lib/lp/services/doc/sprites.txt	2010-07-24 02:27:19 +0000
+++ lib/lp/services/doc/sprites.txt	2012-03-21 02:22:21 +0000
@@ -10,10 +10,10 @@
 
     make sprite_image
 
-The resulting icon-sprites and icon-sprites.positioning files must
-be committed to the repository. Any changes to the CSS template will
-be automatically picked up when Launchpad is restarted. If you don't
-want to restart launchpad.dev, you can run::
+The resulting icon-sprites.png and icon-sprites.positioning files must
+be committed to the repository. Any changes to the CSS template will be
+automatically picked up when Launchpad is restarted. If you don't want
+to restart launchpad.dev, you can run::
 
     make css_combine