fixes #1003 - History is started before navigate

jashkenas · Feb 15, 2012 · 232c888 · 232c888
1 parent 04e2e20
commit 232c888
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 21 deletions.
diff --git a/backbone.js b/backbone.js
@@ -851,7 +851,7 @@
 
   // Handles cross-browser history management, based on URL fragments. If the
   // browser does not support `onhashchange`, falls back to polling.
-  Backbone.History = function() {
+  var History = Backbone.History = function() {
     this.handlers = [];
     _.bindAll(this, 'checkUrl');
   };
@@ -863,10 +863,10 @@
   var isExplorer = /msie [\w.]+/;
 
   // Has the history handling already been started?
-  var historyStarted = false;
+  History.started = false;
 
   // Set up all inheritable **Backbone.History** properties and methods.
-  _.extend(Backbone.History.prototype, Backbone.Events, {
+  _.extend(History.prototype, Backbone.Events, {
 
     // The default interval to poll for hash changes, if necessary, is
     // twenty times a second.
@@ -892,17 +892,19 @@
     // Start the hash change handling, returning `true` if the current URL matches
     // an existing route, and `false` otherwise.
     start: function(options) {
+      if (History.started) throw new Error("Backbone.history has already been started");
+      History.started = true;
 
       // Figure out the initial configuration. Do we need an iframe?
       // Is pushState desired ... is it available?
-      if (historyStarted) throw new Error("Backbone.history has already been started");
       this.options          = _.extend({}, {root: '/'}, this.options, options);
       this._wantsHashChange = this.options.hashChange !== false;
       this._wantsPushState  = !!this.options.pushState;
       this._hasPushState    = !!(this.options.pushState && window.history && window.history.pushState);
       var fragment          = this.getFragment();
       var docMode           = document.documentMode;
       var oldIE             = (isExplorer.exec(navigator.userAgent.toLowerCase()) && (!docMode || docMode <= 7));
+
       if (oldIE) {
         this.iframe = $('<iframe src="javascript:0" tabindex="-1" />').hide().appendTo('body')[0].contentWindow;
         this.navigate(fragment);
@@ -921,7 +923,6 @@
       // Determine if we need to change the base url, for a pushState link
       // opened by a non-pushState browser.
       this.fragment = fragment;
-      historyStarted = true;
       var loc = window.location;
       var atRoot  = loc.pathname == this.options.root;
 
@@ -950,7 +951,7 @@
     stop: function() {
       $(window).unbind('popstate', this.checkUrl).unbind('hashchange', this.checkUrl);
       clearInterval(this._checkUrlInterval);
-      historyStarted = false;
+      History.started = false;
     },
 
     // Add a route to be tested when the fragment changes. Routes added later
@@ -991,7 +992,7 @@
     // route callback be fired (not usually desirable), or `replace: true`, if
     // you wish to modify the current URL without adding an entry to the history.
     navigate: function(fragment, options) {
-      if (!historyStarted) return false;
+      if (!History.started) return false;
       if (!options || options === true) options = {trigger: options};
       var frag = (fragment || '').replace(routeStripper, '');
       if (this.fragment == frag || this.fragment == decodeURIComponent(frag)) return;

diff --git a/test/router.js b/test/router.js
@@ -1,6 +1,32 @@
 $(document).ready(function() {
 
-  module("Backbone.Router");
+  var router = null;
+  var lsatRoute = null;
+  var lastArgs = [];
+
+  function onRoute(router, route, args) {
+    lastRoute = route;
+    lastArgs = args;
+  }
+
+  module("Backbone.Router", {
+
+    setup: function() {
+      Backbone.history = null;
+      router = new Router({testing: 101});
+      Backbone.history.interval = 9;
+      Backbone.history.start({pushState: false});
+      lastRoute = null;
+      lastArgs = [];
+      Backbone.history.on('route', onRoute);
+    },
+
+    teardown: function() {
+      Backbone.history.stop();
+      Backbone.history.off('route', onRoute);
+    }
+
+  });
 
   var Router = Backbone.Router.extend({
 
@@ -58,19 +84,6 @@ $(document).ready(function() {
 
   });
 
-  Backbone.history = null;
-  var router = new Router({testing: 101});
-
-  Backbone.history.interval = 9;
-  Backbone.history.start({pushState: false});
-
-  var lastRoute = null;
-  var lastArgs = [];
-  Backbone.history.bind('route', function(router, route, args) {
-    lastRoute = route;
-    lastArgs = args;
-  });
-
   test("Router: initialize", function() {
     equal(router.testing, 101);
   });
@@ -208,4 +221,11 @@ $(document).ready(function() {
     equal(history.getFragment('/root/foo'), 'foo');
   });
 
+  test("#1003 - History is started before navigate is called", function() {
+    var history = new Backbone.History();
+    history.navigate = function(){ ok(Backbone.History.started); };
+    Backbone.history.stop();
+    history.start();
+  });
+
 });