Permalink
Browse files

Fixes SC.routes bug where browser history events would trigger histor…

…y updates, doubling up the history state. Includes really terrible unit test. Fixes #719.
  • Loading branch information...
1 parent 9ad4ca0 commit ba90943a05587bbd3d9e41524c66e4ce7e20d77f @dcporter dcporter committed Feb 16, 2014
Showing with 54 additions and 11 deletions.
  1. +28 −11 frameworks/routing/system/routes.js
  2. +26 −0 frameworks/routing/tests/system/routes.js
@@ -246,19 +246,24 @@ SC.routes = SC.Object.create(
value = crumbs.route + crumbs.params;
}
- if (!SC.empty(value) || (this._location && this._location !== value)) {
- encodedValue = encodeURI(value);
-
- if (this.usesHistory) {
- if (encodedValue.length > 0) {
- encodedValue = '/' + encodedValue;
+ // Only update the browser if this event triggered from within the app, rather
+ // than from the browser back or forward buttons.
+ if (!this._exogenous) {
+ if (!SC.empty(value) || (this._location && this._location !== value)) {
+ encodedValue = encodeURI(value);
+
+ if (this.usesHistory) {
+ if (encodedValue.length > 0) {
+ encodedValue = '/' + encodedValue;
+ }
+ window.history.pushState(null, null, this.get('baseURI') + encodedValue);
+ } else {
+ window.location.hash = encodedValue;
}
- window.history.pushState(null, null, this.get('baseURI') + encodedValue);
- } else {
- window.location.hash = encodedValue;
}
}
+ // Cache locally.
this._location = value;
}
@@ -310,6 +315,10 @@ SC.routes = SC.Object.create(
if it supports the hashchange event, or by our timer if not.
*/
hashChange: function(event) {
+ // Mark this location change as coming from the browser, which therefore doesn't
+ // need to be updated.
+ this._exogenous = YES;
+
var loc = window.location.hash;
// Remove the '#' prefix
@@ -325,10 +334,16 @@ SC.routes = SC.Object.create(
this.set('location', loc);
}, this);
}
- this._skipRoute = false;
+
+ this._skipRoute = NO;
+ this._exogenous = NO;
},
popState: function(event) {
+ // Mark this location change as coming from the browser, which therefore doesn't
+ // need to be updated.
+ this._exogenous = YES;
+
var base = this.get('baseURI'),
loc = document.location.href;
@@ -343,7 +358,9 @@ SC.routes = SC.Object.create(
}, this);
}
}
- this._skipRoute = false;
+
+ this._skipRoute = NO;
+ this._exogenous = NO;
},
/**
@@ -441,3 +441,29 @@ test('deparam outputs object from string', function() {
{ query: 'test', numItems: 5, size: 'small' },
'deparam works with params only string');
});
+
+
+// For this module, we're going to replace the route's inner plumbing with a test
+// version. Fragile. Apologies.
+var _extractLocation = SC.routes._extractLocation;
+module("Browser events", {
+ teardown: function() {
+ // Reset the innards.
+ SC.routes._extractLocation = _extractLocation;
+ }
+});
+
+test("Internal flag is properly set for browser events.", function() {
+ var runCount = 0;
+ SC.routes._extractLocation = function() {
+ runCount++;
+ ok(this._exogenous, "Internal flag reflects that location is being updated by the browser.");
+ }
+
+ SC.routes.hashChange();
+ if (runCount === 0) ok(false, "Internal plumbing method '_extractLocation' failed to fire as expected.");
+
+ runCount = 0;
+ SC.routes.popState();
+ if (runCount === 0) ok(false, "Internal plumbing method '_extractLocation' failed to fire as expected.");
+});

0 comments on commit ba90943

Please sign in to comment.