launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/mustache-model-undefined-891239 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #891239 in Launchpad itself: "CustomBugListings js error: Cannot read property 'mustache_model'"
  https://bugs.launchpad.net/launchpad/+bug/891239

For more details, see:
https://code.launchpad.net/~deryck/launchpad/mustache-model-undefined-891239/+merge/82463

We were getting an undefined error in Chromium with the new bug listings work because Chrome fires a popstate event on every page load, unlike every other browser.  popstate is what history tools use to know when you're moving forward and backward in history.  Other browsers only fire popstate when you press the back or forward buttons.

The work around was easy enough: inspect e.changed object to see if our history state data is in there.  When it's a page load this check doesn't pass, and the code does nothing.

The idea here is that we only do something on history:change when there's changed data to do something with.
-- 
https://code.launchpad.net/~deryck/launchpad/mustache-model-undefined-891239/+merge/82463
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/mustache-model-undefined-891239 into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2011-11-14 16:10:10 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2011-11-16 22:17:26 +0000
@@ -86,10 +86,12 @@
      * Event handler for history:change events.
      */
     history_changed: function(e){
-        var batch_key = e.newVal.batch_key;
-        var batch = this.get('batches')[batch_key];
-        this.set('current_batch', batch);
-        this.render();
+        if (e.changed.hasOwnProperty('batch_key')) {
+            var batch_key = e.newVal.batch_key;
+            var batch = this.get('batches')[batch_key];
+            this.set('current_batch', batch);
+            this.render();
+        }
     },
 
     /**