From c806afc3ce6d41c88b460569c56c69fddbdd38ce Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Mon, 28 Jul 2014 16:36:29 -0700 Subject: [PATCH 1/4] fix($location): update internal url with browser url if changed outside of Angular $browser keeps an internal representation of the browser's url, and although $browser.url() would return the client's real current url, the $location service that was comparing that url with $location's representation had no way of knowing when the url had been changed outside of Angular. This commit makes browser and location a little bit smarter by setting a flag inside of $browser when it detects that a url changed outside of Angular, allowing $location to parse the new url and update its own internal representation of the url (and react appropriately). Fixes #6976 --- src/ng/browser.js | 38 ++++++++++++++++++++++++++++++++++++-- src/ng/location.js | 16 +++++++++++++++- test/ng/locationSpec.js | 23 +++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index 3ca4a7c0a86c..8a13602cc50b 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -29,7 +29,8 @@ function Browser(window, document, $log, $sniffer) { history = window.history, setTimeout = window.setTimeout, clearTimeout = window.clearTimeout, - pendingDeferIds = {}; + pendingDeferIds = {}, + urlChangedOutsideAngular = false; self.isMock = false; @@ -146,6 +147,7 @@ function Browser(window, document, $log, $sniffer) { * @param {boolean=} replace Should new url replace current history record ? */ self.url = function(url, replace) { + var currentHref; // Android Browser BFCache causes location, history reference to become stale. if (location !== window.location) location = window.location; if (history !== window.history) history = window.history; @@ -175,7 +177,38 @@ function Browser(window, document, $log, $sniffer) { // - newLocation is a workaround for an IE7-9 issue with location.replace and location.href // methods not updating location.href synchronously. // - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172 - return newLocation || location.href.replace(/%27/g,"'"); + if (newLocation) { + return newLocation; + } + if (lastBrowserUrl !== (currentHref = location.href.replace(/%27/g,"'"))) { + urlChangedOutsideAngular = true; + } + return currentHref; + } + }; + + /** + * @name $browser#urlChangedOutsideAngular + * + * @description + * GETTER: + * Without any argument, this method just returns current value of urlChangedOutsideAngular. + * + * SETTER: + * With at least one argument, this method sets urlChangedOutsideAngular to new value. + * + * NOTE: this api is intended for use only by the $location service inside of $locationWatch. + * + * @param {boolean} val New value to set as urlChangedOutsideAngular, + * typically used to reset value to false. + */ + self.urlChangedOutsideAngular = function(val) { + if (isDefined(val)) { + urlChangedOutsideAngular = val; + return self; + } + else { + return urlChangedOutsideAngular; } }; @@ -184,6 +217,7 @@ function Browser(window, document, $log, $sniffer) { function fireUrlChange() { newLocation = null; + urlChangedOutsideAngular = false; if (lastBrowserUrl == self.url()) return; lastBrowserUrl = self.url(); diff --git a/src/ng/location.js b/src/ng/location.js index 98dec092dd26..46e1c27385ea 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -744,7 +744,21 @@ function $LocationProvider(){ if (!changeCounter || oldUrl != $location.absUrl()) { changeCounter++; $rootScope.$evalAsync(function() { - if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), oldUrl). + /** + * $browser should detect if url was changed outside of Angular, in which + * case $location should automatically update to the new url. + * + * NOTE: Detecting outside changes to location happens automatically in $browser via + * window events (or polling if events aren't supported), but those methods are + * async. For this reason, $browser.url() will perform a comparison with each call + * to the method as a getter. + * NOTE: Method does not exist on mock $browser + */ + if ($browser.urlChangedOutsideAngular && $browser.urlChangedOutsideAngular()) { + $location.$$parse(oldUrl); + $browser.urlChangedOutsideAngular(false); + } + else if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), oldUrl). defaultPrevented) { $location.$$parse(oldUrl); } else { diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index ff36ce42a888..16623c98773d 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -769,6 +769,29 @@ describe('$location', function() { }); + it('should update location when location changed outside of Angular', function() { + module(function($windowProvider, $locationProvider, $browserProvider) { + $locationProvider.html5Mode(true); + $browserProvider.$get = function($document, $window, $log, $sniffer) { + var b = new Browser($window, $document, $log, $sniffer); + b.pollFns = []; + return b; + }; + }); + + inject(function($rootScope, $browser, $location, $sniffer){ + if ($sniffer.history) { + window.history.replaceState(null, '', '/hello'); + // Verify that infinite digest reported in #6976 no longer occurs + expect(function() { + $rootScope.$digest(); + }).not.toThrow(); + expect($location.path()).toBe('/hello'); + } + }); + }); + + it('should rewrite when hashbang url given', function() { initService(true, '!', true); inject( From d18cb2a3609c77ff24db381fc31fbf83b8e302d6 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Tue, 26 Aug 2014 12:49:23 -0700 Subject: [PATCH 2/4] WIP: rename local hasChangedOutside variable to not match getter/setter name --- src/ng/browser.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index 8a13602cc50b..528a1a9899e4 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -30,7 +30,7 @@ function Browser(window, document, $log, $sniffer) { setTimeout = window.setTimeout, clearTimeout = window.clearTimeout, pendingDeferIds = {}, - urlChangedOutsideAngular = false; + hasChangedOutside = false; self.isMock = false; @@ -181,7 +181,7 @@ function Browser(window, document, $log, $sniffer) { return newLocation; } if (lastBrowserUrl !== (currentHref = location.href.replace(/%27/g,"'"))) { - urlChangedOutsideAngular = true; + hasChangedOutside = true; } return currentHref; } @@ -202,13 +202,13 @@ function Browser(window, document, $log, $sniffer) { * @param {boolean} val New value to set as urlChangedOutsideAngular, * typically used to reset value to false. */ - self.urlChangedOutsideAngular = function(val) { - if (isDefined(val)) { - urlChangedOutsideAngular = val; + self.urlChangedOutsideAngular = function(hasChanged) { + if (isDefined(hasChanged)) { + hasChangedOutside = hasChanged; return self; } else { - return urlChangedOutsideAngular; + return hasChangedOutside; } }; @@ -217,7 +217,7 @@ function Browser(window, document, $log, $sniffer) { function fireUrlChange() { newLocation = null; - urlChangedOutsideAngular = false; + hasChangedOutside = false; if (lastBrowserUrl == self.url()) return; lastBrowserUrl = self.url(); From 5ee60c987aec37500e1be41ecae348cc70e9ad37 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Tue, 26 Aug 2014 12:53:16 -0700 Subject: [PATCH 3/4] WIP: rename oldUrl to browserUrl to better represent context --- src/ng/location.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index 46e1c27385ea..bb73fd96f2d6 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -738,10 +738,10 @@ function $LocationProvider(){ // update browser var changeCounter = 0; $rootScope.$watch(function $locationWatch() { - var oldUrl = $browser.url(); + var browserUrl = $browser.url(); var currentReplace = $location.$$replace; - if (!changeCounter || oldUrl != $location.absUrl()) { + if (!changeCounter || browserUrl != $location.absUrl()) { changeCounter++; $rootScope.$evalAsync(function() { /** @@ -755,15 +755,15 @@ function $LocationProvider(){ * NOTE: Method does not exist on mock $browser */ if ($browser.urlChangedOutsideAngular && $browser.urlChangedOutsideAngular()) { - $location.$$parse(oldUrl); + $location.$$parse(browserUrl); $browser.urlChangedOutsideAngular(false); } - else if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), oldUrl). + else if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), browserUrl). defaultPrevented) { - $location.$$parse(oldUrl); + $location.$$parse(browserUrl); } else { $browser.url($location.absUrl(), currentReplace); - afterLocationChange(oldUrl); + afterLocationChange(browserUrl); } }); } From c62f204a5e41989057fa44837a903f6764f6ae32 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Tue, 26 Aug 2014 12:59:47 -0700 Subject: [PATCH 4/4] WIP: make mock $browser symmetrical with real $browser --- src/ng/location.js | 3 +-- src/ngMock/angular-mocks.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index bb73fd96f2d6..463d8f82fa63 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -752,9 +752,8 @@ function $LocationProvider(){ * window events (or polling if events aren't supported), but those methods are * async. For this reason, $browser.url() will perform a comparison with each call * to the method as a getter. - * NOTE: Method does not exist on mock $browser */ - if ($browser.urlChangedOutsideAngular && $browser.urlChangedOutsideAngular()) { + if ($browser.urlChangedOutsideAngular()) { $location.$$parse(browserUrl); $browser.urlChangedOutsideAngular(false); } diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index ba9790539ff8..55feee371d07 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -35,6 +35,7 @@ angular.mock.$Browser = function() { self.$$url = "http://server/"; self.$$lastUrl = self.$$url; // used by url polling fn self.pollFns = []; + self.$$hasChangedOutside = false; // TODO(vojta): remove this temporary api self.$$completeOutstandingRequest = angular.noop; @@ -68,6 +69,16 @@ angular.mock.$Browser = function() { return self.deferredNextId++; }; + self.urlChangedOutsideAngular = function (hasChanged) { + if (isDefined(hasChanged)) { + self.$$hasChangedOutside = hasChanged; + return self; + } + else { + return self.$$hasChangedOutside; + } + }; + /** * @name $browser#defer.now @@ -148,6 +159,10 @@ angular.mock.$Browser.prototype = { return this; } + if (this.$$mockLocation && this.$$mockLocation.href && url !== (this.$$url = this.$$mockLocation.href.replace(/%27/g,"'"))) { + this.hasChangedOutside = true; + } + return this.$$url; },