From 49fa2b1d2910cd06fe09624ae8459fe4b24cb2ed Mon Sep 17 00:00:00 2001 From: hamfastgamgee <hamfastgamgee@gmail.com> Date: Tue, 14 Apr 2015 14:06:22 -0500 Subject: [PATCH 1/5] fix($browser): prevent infinite $digest from no trailing slash in IE9 fix($browser): prevent infinite $digest from no trailing slash in IE9 This fix prevents IE9 from throwing an infinite $digest error when the user accesses the base URL of the site without a trailing slash. Suppose you owned http://www.mysite.com/app and had an Angular app hosted in a subdirectory "app". If an IE9 user accessed http://www.mysite.com/app infinite $digest errors would be thrown on the console, but the app itself would eventually resolve properly and work fine. Now the infinite $digest errors will not be thrown. Closes #11439 --- src/ng/browser.js | 20 ++ src/ng/location.js | 11 +- test/ngRoute/routeSpec.js | 377 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 407 insertions(+), 1 deletion(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index 8bd6b424bff8..fbebe01b3b8b 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -135,6 +135,26 @@ function Browser(window, document, $log, $sniffer) { cacheState(); lastHistoryState = cachedState; + /** + * @name $browser#forceReloadLocationUpdate + * + * @description + * This method is a setter. + * + * If the reloadLocation variable is already set, it will be reset to + * the passed-in URL. + * + * NOTE: this api is intended for use only by the $location service in the + * $locationWatch function. + * + * @param {string} url New url + */ + self.forceReloadLocationUpdate = function(url) { + if (reloadLocation) { + reloadLocation = url; + } + }; + /** * @name $browser#url * diff --git a/src/ng/location.js b/src/ng/location.js index ffee4cd4820f..77edb6c2a018 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -883,7 +883,7 @@ function $LocationProvider() { $browser.url($location.absUrl(), true); } - var initializing = true; + var initializing = true, previousOldUrl = null, previousNewUrl = null; // update $location when $browser url changes $browser.onUrlChange(function(newUrl, newState) { @@ -918,6 +918,15 @@ function $LocationProvider() { $rootScope.$watch(function $locationWatch() { var oldUrl = trimEmptyHash($browser.url()); var newUrl = trimEmptyHash($location.absUrl()); + if ($location.$$html5 && !$sniffer.history) { + if (previousOldUrl === oldUrl && previousNewUrl === newUrl) { + // break out of infinite $digest loops caused by default routes in hashbang mode + $browser.forceReloadLocationUpdate(newUrl); + previousOldUrl = previousNewUrl = null; + return; + } + previousOldUrl = oldUrl, previousNewUrl = newUrl; + } var oldState = $browser.state(); var currentReplace = $location.$$replace; var urlOrStateChanged = oldUrl !== newUrl || diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 368dadf3c9ae..d4f502689bc3 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1037,6 +1037,383 @@ describe('$route', function() { }); }); + function initBrowserForInfiniteDigestTests() { + return function($browser) { + $browser.url = function(url, replace, state) { + if (angular.isUndefined(state)) { + state = null; + } + if (url) { + if (this.$$lastUrl === url) { + return this; + } + + var index = this.$$lastUrl.indexOf('#'); + var lastUrlStripped = (index === -1 ? this.$$lastUrl : this.$$lastUrl.substr(0, index)); + index = url.indexOf('#'); + var urlStripped = (index === -1 ? url : url.substr(0, index)); + + var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; + if (!sameBase) { + this.$$reloadLocation = url; + } + this.$$url = url; + this.$$lastUrl = url; + // Native pushState serializes & copies the object; simulate it. + this.$$state = angular.copy(state); + return this; + } + + return this.$$reloadLocation || this.$$url; + }; + $browser.$$lastUrl = 'http://server/'; + $browser.$$baseHref = '/app/'; + $browser.forceReloadLocationUpdate = function(url) { + if (this.$$reloadLocation) { + this.$$reloadLocation = url; + } + }; + }; + } + + function initLocationAndRouteServices(options) { + return module(function($provide, $locationProvider, $routeProvider) { + $locationProvider.html5Mode(options.html5Mode); + $provide.value('$sniffer', {history: options.history}); + + $routeProvider. + when(options.matchingRoute, { + templateUrl: 'foo.html', + }). + otherwise({ + redirectTo: options.otherwiseRoute + }); + }); + } + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/Home', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with home route without trailing slash in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without home route without trailing slash in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without trailing slash when otherwise route shouldn\'t be called in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/Home', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with home route with trailing slash in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without home route with trailing slash in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for hashbang browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with trailing slash when otherwise route shouldn\'t be called in non-history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/Home', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with home route without trailing slash in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without home route without trailing slash in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without trailing slash when otherwise route shouldn\'t be called in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/Home', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with home route with trailing slash in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without home route with trailing slash in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(1); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); + + describe('location watch for HTML5 browsers with routing taken into account', function() { + beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/Home'})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL with trailing slash when otherwise route shouldn\'t be called in history browser', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + + var $location = $injector.get('$location'); + var $route = $injector.get('$route'); + + $browser.poll(); + + $rootScope.$digest(); + + expect($route.current.loadedTemplateUrl).toEqual('foo.html'); + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(1); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); describe('reloadOnSearch', function() { it('should reload a route when reloadOnSearch is enabled and .search() changes', function() { From cd8f42e3d1d1bcadb9caf07d9d08001ee081bcb5 Mon Sep 17 00:00:00 2001 From: hamfastgamgee <hamfastgamgee@gmail.com> Date: Wed, 22 Apr 2015 07:18:23 -0500 Subject: [PATCH 2/5] make jscs happy --- test/ngRoute/routeSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index d4f502689bc3..5a06a9e8a374 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1083,7 +1083,7 @@ describe('$route', function() { $routeProvider. when(options.matchingRoute, { - templateUrl: 'foo.html', + templateUrl: 'foo.html' }). otherwise({ redirectTo: options.otherwiseRoute From fa24709aeba01d3b25c63738b7107d4e34b81477 Mon Sep 17 00:00:00 2001 From: hamfastgamgee <hamfastgamgee@gmail.com> Date: Thu, 21 May 2015 11:40:55 -0500 Subject: [PATCH 3/5] changes after review by petebacondarwin --- test/ng/locationSpec.js | 224 ++++++++++++++++++++++ test/ngRoute/routeSpec.js | 377 -------------------------------------- 2 files changed, 224 insertions(+), 377 deletions(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index de0561d0c16f..035cd206c256 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -2278,4 +2278,228 @@ describe('$location', function() { throwOnState(location); }); }); + + + function initBrowserForInfiniteDigestTests() { + return function($browser) { + $browser.url = function(url, replace, state) { + if (angular.isUndefined(state)) { + state = null; + } + if (url) { + if (this.$$lastUrl === url) { + return this; + } + + var index = this.$$lastUrl.indexOf('#'); + var lastUrlStripped = (index === -1 ? this.$$lastUrl : this.$$lastUrl.substr(0, index)); + index = url.indexOf('#'); + var urlStripped = (index === -1 ? url : url.substr(0, index)); + + var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; + if (!sameBase) { + this.$$reloadLocation = url; + } + this.$$url = url; + this.$$lastUrl = url; + // Native pushState serializes & copies the object; simulate it. + this.$$state = angular.copy(state); + return this; + } + + return this.$$reloadLocation || this.$$url; + }; + $browser.$$lastUrl = 'http://server/'; + $browser.$$baseHref = '/app/'; + $browser.forceReloadLocationUpdate = function(url) { + if (this.$$reloadLocation) { + this.$$reloadLocation = url; + } + }; + }; + } + + function initLocationServices(options) { + return module(function($provide, $locationProvider) { + $locationProvider.html5Mode(options.html5Mode); + $provide.value('$sniffer', {history: options.history}); + }); + } + + function initChangeSuccessListener($rootScope, $location, newPath) { + $rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) { + $location.path(newPath); + }); + } + + describe('location watch for hashbang browsers', function() { + beforeEach(initLocationServices({html5Mode: true, history: false})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/Home'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/#/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + + it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + + it('should not infinite $digest when going to base URL with trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/Home'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/#/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + + it('should not infinite $digest when going to base URL with trailing slash when $locationChangeSuccess watcher changes path to /', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/#/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); + }); + }); + }); + + + describe('location watch for HTML5 browsers', function() { + beforeEach(initLocationServices({html5Mode: true, history: true})); + beforeEach(inject(initBrowserForInfiniteDigestTests())); + + it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/Home'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(3); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + + it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + + it('should not infinite $digest when going to base URL with trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/Home'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/Home'); + expect($location.path()).toEqual('/Home'); + expect($browserUrl.calls.length).toEqual(2); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + + it('should not infinite $digest when going to base URL with trailing slash when $locationChangeSuccess watcher changes path to /', function() { + inject(function($rootScope, $injector, $browser) { + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); + + $browser.url('http://server/app/'); + $browser.poll(); + + var $location = $injector.get('$location'); + initChangeSuccessListener($rootScope, $location, '/'); + + $rootScope.$digest(); + + expect($browser.url()).toEqual('http://server/app/'); + expect($location.path()).toEqual('/'); + expect($browserUrl.calls.length).toEqual(1); + expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 5a06a9e8a374..368dadf3c9ae 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1037,383 +1037,6 @@ describe('$route', function() { }); }); - function initBrowserForInfiniteDigestTests() { - return function($browser) { - $browser.url = function(url, replace, state) { - if (angular.isUndefined(state)) { - state = null; - } - if (url) { - if (this.$$lastUrl === url) { - return this; - } - - var index = this.$$lastUrl.indexOf('#'); - var lastUrlStripped = (index === -1 ? this.$$lastUrl : this.$$lastUrl.substr(0, index)); - index = url.indexOf('#'); - var urlStripped = (index === -1 ? url : url.substr(0, index)); - - var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; - if (!sameBase) { - this.$$reloadLocation = url; - } - this.$$url = url; - this.$$lastUrl = url; - // Native pushState serializes & copies the object; simulate it. - this.$$state = angular.copy(state); - return this; - } - - return this.$$reloadLocation || this.$$url; - }; - $browser.$$lastUrl = 'http://server/'; - $browser.$$baseHref = '/app/'; - $browser.forceReloadLocationUpdate = function(url) { - if (this.$$reloadLocation) { - this.$$reloadLocation = url; - } - }; - }; - } - - function initLocationAndRouteServices(options) { - return module(function($provide, $locationProvider, $routeProvider) { - $locationProvider.html5Mode(options.html5Mode); - $provide.value('$sniffer', {history: options.history}); - - $routeProvider. - when(options.matchingRoute, { - templateUrl: 'foo.html' - }). - otherwise({ - redirectTo: options.otherwiseRoute - }); - }); - } - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/Home', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with home route without trailing slash in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/Home'); - expect($location.path()).toEqual('/Home'); - expect($browserUrl.calls.length).toEqual(3); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without home route without trailing slash in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(3); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without trailing slash when otherwise route shouldn\'t be called in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(3); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/Home', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with home route with trailing slash in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/Home'); - expect($location.path()).toEqual('/Home'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without home route with trailing slash in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for hashbang browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: false, matchingRoute: '/', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with trailing slash when otherwise route shouldn\'t be called in non-history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/#/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).toHaveBeenCalledOnce(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/Home', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with home route without trailing slash in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/Home'); - expect($location.path()).toEqual('/Home'); - expect($browserUrl.calls.length).toEqual(3); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without home route without trailing slash in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without trailing slash when otherwise route shouldn\'t be called in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/Home', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with home route with trailing slash in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/Home'); - expect($location.path()).toEqual('/Home'); - expect($browserUrl.calls.length).toEqual(2); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL without home route with trailing slash in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(1); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); - - describe('location watch for HTML5 browsers with routing taken into account', function() { - beforeEach(initLocationAndRouteServices({html5Mode: true, history: true, matchingRoute: '/', otherwiseRoute: '/Home'})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); - - it('should not infinite $digest when going to base URL with trailing slash when otherwise route shouldn\'t be called in history browser', function() { - inject(function($rootScope, $injector, $browser) { - var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); - var $browserForceReloadLocationUpdate = spyOn($browser, 'forceReloadLocationUpdate').andCallThrough(); - - $browser.url('http://server/app/'); - - var $location = $injector.get('$location'); - var $route = $injector.get('$route'); - - $browser.poll(); - - $rootScope.$digest(); - - expect($route.current.loadedTemplateUrl).toEqual('foo.html'); - expect($browser.url()).toEqual('http://server/app/'); - expect($location.path()).toEqual('/'); - expect($browserUrl.calls.length).toEqual(1); - expect($browserForceReloadLocationUpdate).not.toHaveBeenCalled(); - }); - }); - }); describe('reloadOnSearch', function() { it('should reload a route when reloadOnSearch is enabled and .search() changes', function() { From d519077b3bd31e0d98147beb4e8f575b30f8597d Mon Sep 17 00:00:00 2001 From: hamfastgamgee <hamfastgamgee@gmail.com> Date: Sat, 23 May 2015 07:00:23 -0500 Subject: [PATCH 4/5] modify mock browser to model actual $browser also fix downstream impacts and tests --- src/ng/location.js | 56 +++++++++++------------ src/ngMock/angular-mocks.js | 43 ++++++++++++++---- test/ng/locationSpec.js | 91 ++++++++++++++----------------------- 3 files changed, 98 insertions(+), 92 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index 6095cae4a70a..d8a76cec3762 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -926,15 +926,6 @@ function $LocationProvider() { $rootScope.$watch(function $locationWatch() { var oldUrl = trimEmptyHash($browser.url()); var newUrl = trimEmptyHash($location.absUrl()); - if ($location.$$html5 && !$sniffer.history) { - if (previousOldUrl === oldUrl && previousNewUrl === newUrl) { - // break out of infinite $digest loops caused by default routes in hashbang mode - $browser.forceReloadLocationUpdate(newUrl); - previousOldUrl = previousNewUrl = null; - return; - } - previousOldUrl = oldUrl, previousNewUrl = newUrl; - } var oldState = $browser.state(); var currentReplace = $location.$$replace; var urlOrStateChanged = oldUrl !== newUrl || @@ -943,26 +934,35 @@ function $LocationProvider() { if (initializing || urlOrStateChanged) { initializing = false; - $rootScope.$evalAsync(function() { - var newUrl = $location.absUrl(); - var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl, - $location.$$state, oldState).defaultPrevented; - - // if the location was changed by a `$locationChangeStart` handler then stop - // processing this location change - if ($location.absUrl() !== newUrl) return; - - if (defaultPrevented) { - $location.$$parse(oldUrl); - $location.$$state = oldState; - } else { - if (urlOrStateChanged) { - setBrowserUrlWithFallback(newUrl, currentReplace, - oldState === $location.$$state ? null : $location.$$state); + if ((previousOldUrl !== oldUrl) || (previousNewUrl !== newUrl)) { + previousOldUrl = oldUrl, previousNewUrl = newUrl; + + $rootScope.$evalAsync(function() { + var newUrl = $location.absUrl(); + var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl, + $location.$$state, oldState).defaultPrevented; + + // if the location was changed by a `$locationChangeStart` handler then stop + // processing this location change + if ($location.absUrl() !== newUrl) return; + + if (defaultPrevented) { + $location.$$parse(oldUrl); + $location.$$state = oldState; + } else { + if (urlOrStateChanged) { + setBrowserUrlWithFallback(newUrl, currentReplace, + oldState === $location.$$state ? null : $location.$$state); + } + afterLocationChange(oldUrl, oldState); } - afterLocationChange(oldUrl, oldState); - } - }); + }); + } else { + $browser.forceReloadLocationUpdate(newUrl); + previousOldUrl = previousNewUrl = false; + } + } else { + previousOldUrl = previousNewUrl = false; } $location.$$replace = false; diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 562f88668966..f5d81ad504c7 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -23,17 +23,19 @@ angular.mock = {}; * that there are several helper methods available which can be used in tests. */ angular.mock.$BrowserProvider = function() { - this.$get = function() { - return new angular.mock.$Browser(); - }; + this.$get = ['$sniffer', function($sniffer) { + return new angular.mock.$Browser($sniffer); + }]; }; -angular.mock.$Browser = function() { +angular.mock.$Browser = function($sniffer) { var self = this; this.isMock = true; self.$$url = "http://server/"; self.$$lastUrl = self.$$url; // used by url polling fn + self.$$reloadLocation = null; + self.$$sniffer = $sniffer; self.pollFns = []; // TODO(vojta): remove this temporary api @@ -150,20 +152,39 @@ angular.mock.$Browser.prototype = { state = null; } if (url) { + var sameState = this.$$state === state; + + if (this.$$lastUrl === url && (!this.$$sniffer.history || sameState)) { + return this; + } + + var index = this.$$lastUrl.indexOf('#'); + var lastUrlStripped = (index === -1 ? this.$$lastUrl : this.$$lastUrl.substr(0, index)); + index = url.indexOf('#'); + var urlStripped = (index === -1 ? url : url.substr(0, index)); + + var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; + if (this.$$sniffer.history && (!sameBase || !sameState)) { + // Native pushState serializes & copies the object; simulate it. + this.$$state = angular.copy(state); + } + if (!sameBase) { + this.$$reloadLocation = url; + } this.$$url = url; - // Native pushState serializes & copies the object; simulate it. - this.$$state = angular.copy(state); + this.$$lastUrl = url; + return this; } - return this.$$url; + return this.$$reloadLocation || this.$$url; }, state: function() { return this.$$state; }, - cookies: function(name, value) { + cookies: function(name, value) { if (name) { if (angular.isUndefined(value)) { delete this.cookieHash[name]; @@ -184,6 +205,12 @@ angular.mock.$Browser.prototype = { notifyWhenNoOutstandingRequests: function(fn) { fn(); + }, + + forceReloadLocationUpdate: function(url) { + if (this.$$reloadLocation) { + this.$$reloadLocation = url; + } } }; diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 035cd206c256..c2ea57e588b2 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -650,13 +650,17 @@ describe('$location', function() { function initService(options) { return module(function($provide, $locationProvider) { $locationProvider.html5Mode(options.html5Mode); - $locationProvider.hashPrefix(options.hashPrefix); + if (options.hashPrefix) { + $locationProvider.hashPrefix(options.hashPrefix); + } $provide.value('$sniffer', {history: options.supportHistory}); }); } function initBrowser(options) { return function($browser) { - $browser.url(options.url); + if (options.url) { + $browser.url(options.url); + } $browser.$$baseHref = options.basePath; }; } @@ -702,7 +706,7 @@ describe('$location', function() { // location.href = '...' fires hashchange event synchronously, so it might happen inside $apply it('should not $apply when browser url changed inside $apply', inject( function($rootScope, $browser, $location) { - var OLD_URL = $browser.url(), + var OLD_URL = $location.absUrl(), NEW_URL = 'http://new.com/a/b#!/new'; @@ -718,7 +722,7 @@ describe('$location', function() { // location.href = '...' fires hashchange event synchronously, so it might happen inside $digest it('should not $apply when browser url changed inside $digest', inject( function($rootScope, $browser, $location) { - var OLD_URL = $browser.url(), + var OLD_URL = $location.absUrl(), NEW_URL = 'http://new.com/a/b#!/new', notRunYet = true; @@ -756,7 +760,12 @@ describe('$location', function() { }); $rootScope.$apply(); - expect($browserUrl).toHaveBeenCalledOnce(); + + // This test actually triggers a $digest loop inside the $locationWatch function in $location. + // The first $browser.url() set doesn't reset $browser.url()'s caching variable appropriately, + // so after the second call to it, logic kicks in to force an update of the caching variable. + // Hence, two calls to $browser.url()'s setter is exactly correct here. + expect($browserUrl.calls.length).toBe(2); expect($browser.url()).toBe('http://new.com/a/b#!/new/path?a=b'); })); @@ -993,7 +1002,7 @@ describe('$location', function() { inject( initBrowser({url:'http://domain.com/base/index.html',basePath: '/base/index.html'}), function($browser, $location) { - expect($browser.url()).toBe('http://domain.com/base/index.html#!/index.html'); + expect($location.absUrl()).toBe('http://domain.com/base/index.html#!/index.html'); } ); }); @@ -2280,52 +2289,6 @@ describe('$location', function() { }); - function initBrowserForInfiniteDigestTests() { - return function($browser) { - $browser.url = function(url, replace, state) { - if (angular.isUndefined(state)) { - state = null; - } - if (url) { - if (this.$$lastUrl === url) { - return this; - } - - var index = this.$$lastUrl.indexOf('#'); - var lastUrlStripped = (index === -1 ? this.$$lastUrl : this.$$lastUrl.substr(0, index)); - index = url.indexOf('#'); - var urlStripped = (index === -1 ? url : url.substr(0, index)); - - var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; - if (!sameBase) { - this.$$reloadLocation = url; - } - this.$$url = url; - this.$$lastUrl = url; - // Native pushState serializes & copies the object; simulate it. - this.$$state = angular.copy(state); - return this; - } - - return this.$$reloadLocation || this.$$url; - }; - $browser.$$lastUrl = 'http://server/'; - $browser.$$baseHref = '/app/'; - $browser.forceReloadLocationUpdate = function(url) { - if (this.$$reloadLocation) { - this.$$reloadLocation = url; - } - }; - }; - } - - function initLocationServices(options) { - return module(function($provide, $locationProvider) { - $locationProvider.html5Mode(options.html5Mode); - $provide.value('$sniffer', {history: options.history}); - }); - } - function initChangeSuccessListener($rootScope, $location, newPath) { $rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) { $location.path(newPath); @@ -2333,8 +2296,8 @@ describe('$location', function() { } describe('location watch for hashbang browsers', function() { - beforeEach(initLocationServices({html5Mode: true, history: false})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); + beforeEach(initService({html5Mode: true, supportHistory: false})); + beforeEach(inject(initBrowser({basePath: '/app/'}))); it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { inject(function($rootScope, $injector, $browser) { @@ -2419,8 +2382,8 @@ describe('$location', function() { describe('location watch for HTML5 browsers', function() { - beforeEach(initLocationServices({html5Mode: true, history: true})); - beforeEach(inject(initBrowserForInfiniteDigestTests())); + beforeEach(initService({html5Mode: true, supportHistory: true})); + beforeEach(inject(initBrowser({basePath: '/app/'}))); it('should not infinite $digest when going to base URL without trailing slash when $locationChangeSuccess watcher changes path to /Home', function() { inject(function($rootScope, $injector, $browser) { @@ -2502,4 +2465,20 @@ describe('$location', function() { }); }); }); + + + it('should not get caught in infinite digest when replacing empty path with slash', function() { + initService({html5Mode:true,supportHistory:false}); + initBrowser({url:'http://server/base', basePath:'/base/'}); + inject( + function($browser, $location, $rootScope, $window) { + $rootScope.$on('$locationChangeSuccess', function() { + if ($location.path() !== '/') { + $location.path('/').replace(); + } + }); + $rootScope.$digest(); + } + ); + }); }); From 28d8f10e6b9c84c436cb3c3d5284e07be51538fc Mon Sep 17 00:00:00 2001 From: hamfastgamgee <hamfastgamgee@gmail.com> Date: Sat, 23 May 2015 22:44:43 -0500 Subject: [PATCH 5/5] fix tests --- src/ngMock/angular-mocks.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index f5d81ad504c7..c0af60b23012 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -36,6 +36,7 @@ angular.mock.$Browser = function($sniffer) { self.$$lastUrl = self.$$url; // used by url polling fn self.$$reloadLocation = null; self.$$sniffer = $sniffer; + self.$$simulateLocationUpdate = false; self.pollFns = []; // TODO(vojta): remove this temporary api @@ -48,9 +49,11 @@ angular.mock.$Browser = function($sniffer) { self.onUrlChange = function(listener) { self.pollFns.push( function() { - if (self.$$lastUrl !== self.$$url || self.$$state !== self.$$lastState) { - self.$$lastUrl = self.$$url; + var currUrl = self.$$reloadLocation || self.$$url; + if (self.$$simulateLocationUpdate || self.$$lastUrl !== currUrl || self.$$state !== self.$$lastState) { + self.$$lastUrl = currUrl; self.$$lastState = self.$$state; + self.$$simulateLocationUpdate = false; listener(self.$$url, self.$$state); } } @@ -152,7 +155,7 @@ angular.mock.$Browser.prototype = { state = null; } if (url) { - var sameState = this.$$state === state; + var sameState = this.$$lastState === state; if (this.$$lastUrl === url && (!this.$$sniffer.history || sameState)) { return this; @@ -163,16 +166,20 @@ angular.mock.$Browser.prototype = { index = url.indexOf('#'); var urlStripped = (index === -1 ? url : url.substr(0, index)); + this.$$lastState = angular.copy(state); + var sameBase = this.$$lastUrl && lastUrlStripped === urlStripped; if (this.$$sniffer.history && (!sameBase || !sameState)) { // Native pushState serializes & copies the object; simulate it. this.$$state = angular.copy(state); + this.$$lastState = this.$$state; } if (!sameBase) { this.$$reloadLocation = url; } this.$$url = url; this.$$lastUrl = url; + this.$$simulateLocationUpdate = true; return this; }