diff --git a/src/ng/browser.js b/src/ng/browser.js index 2f1494c905bf..699602bc9b2d 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -1,5 +1,14 @@ 'use strict'; -/* global stripHash: true */ +/* global getHash: true, stripHash: false */ + +function getHash(url) { + var index = url.indexOf('#'); + return index === -1 ? '' : url.substr(index); +} + +function trimEmptyHash(url) { + return url.replace(/#$/, ''); +} /** * ! This is a private undocumented service ! @@ -62,30 +71,26 @@ function Browser(window, document, $log, $sniffer, $$taskTrackerFactory) { cacheState(); - function getHash(url) { - var index = url.indexOf('#'); - return index === -1 ? '' : url.substr(index); - } - /** * @name $browser#url * * @description * GETTER: - * Without any argument, this method just returns current value of location.href. + * Without any argument, this method just returns current value of `location.href` (with a + * trailing `#` stripped of if the hash is empty). * * SETTER: * With at least one argument, this method sets url to new value. - * If html5 history api supported, pushState/replaceState is used, otherwise - * location.href/location.replace is used. - * Returns its own instance to allow chaining + * If html5 history api supported, `pushState`/`replaceState` is used, otherwise + * `location.href`/`location.replace` is used. + * Returns its own instance to allow chaining. * - * NOTE: this api is intended for use only by the $location service. Please use the + * NOTE: this api is intended for use only by the `$location` service. Please use the * {@link ng.$location $location service} to change url. * * @param {string} url New url (when used as setter) * @param {boolean=} replace Should new url replace current history record? - * @param {object=} state object to use with pushState/replaceState + * @param {object=} state State object to use with `pushState`/`replaceState` */ self.url = function(url, replace, state) { // In modern browsers `history.state` is `null` by default; treating it separately @@ -143,7 +148,7 @@ function Browser(window, document, $log, $sniffer, $$taskTrackerFactory) { // - pendingLocation is needed as browsers don't allow to read out // the new location.href if a reload happened or if there is a bug like in iOS 9 (see // https://openradar.appspot.com/22186109). - return pendingLocation || location.href; + return trimEmptyHash(pendingLocation || location.href); } }; diff --git a/src/ng/location.js b/src/ng/location.js index cc064becd727..3904c7290e77 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -1,4 +1,5 @@ 'use strict'; +/* global stripHash: true */ var PATH_MATCH = /^([^?#]*)(\?([^#]*))?(#(.*))?$/, DEFAULT_PORTS = {'http': 80, 'https': 443, 'ftp': 21}; @@ -94,17 +95,11 @@ function stripBaseUrl(base, url) { } } - function stripHash(url) { var index = url.indexOf('#'); return index === -1 ? url : url.substr(0, index); } -function trimEmptyHash(url) { - return url.replace(/(#.+)|#$/, '$1'); -} - - function stripFile(url) { return url.substr(0, stripHash(url).lastIndexOf('/') + 1); } @@ -943,7 +938,7 @@ function $LocationProvider() { // rewrite hashbang url <> html5 url - if (trimEmptyHash($location.absUrl()) !== trimEmptyHash(initialUrl)) { + if ($location.absUrl() !== initialUrl) { $browser.url($location.absUrl(), true); } @@ -962,7 +957,6 @@ function $LocationProvider() { var oldUrl = $location.absUrl(); var oldState = $location.$$state; var defaultPrevented; - newUrl = trimEmptyHash(newUrl); $location.$$parse(newUrl); $location.$$state = newState; @@ -990,8 +984,8 @@ function $LocationProvider() { if (initializing || $location.$$urlUpdatedByLocation) { $location.$$urlUpdatedByLocation = false; - var oldUrl = trimEmptyHash($browser.url()); - var newUrl = trimEmptyHash($location.absUrl()); + var oldUrl = $browser.url(); + var newUrl = $location.absUrl(); var oldState = $browser.state(); var currentReplace = $location.$$replace; var urlOrStateChanged = !urlsEqual(oldUrl, newUrl) || diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 0352bac958c3..20dc9b2e8c9b 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -225,7 +225,8 @@ angular.mock.$Browser.prototype = { state = null; } if (url) { - this.$$url = url; + // The `$browser` service trims empty hashes; simulate it. + this.$$url = url.replace(/#$/, ''); // Native pushState serializes & copies the object; simulate it. this.$$state = angular.copy(state); return this; diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index acaf63e18d23..07f3683b76ad 100644 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -404,6 +404,14 @@ describe('browser', function() { expect(browser.url()).toEqual('https://another.com'); }); + it('should strip an empty hash fragment', function() { + fakeWindow.location.href = 'http://test.com#'; + expect(browser.url()).toEqual('http://test.com'); + + fakeWindow.location.href = 'https://another.com#foo'; + expect(browser.url()).toEqual('https://another.com#foo'); + }); + it('should use history.pushState when available', function() { sniffer.history = true; browser.url('http://new.org'); @@ -1047,6 +1055,32 @@ describe('browser', function() { expect($location.absUrl()).toEqual('http://server/#otherHash'); }); }); + + // issue #16632 + it('should not trigger `$locationChangeStart` more than once due to trailing `#`', function() { + setup({ + history: true, + html5Mode: true + }); + + inject(function($flushPendingTasks, $location, $rootScope) { + $rootScope.$digest(); + + var spy = jasmine.createSpy('$locationChangeStart'); + $rootScope.$on('$locationChangeStart', spy); + + $rootScope.$evalAsync(function() { + fakeWindow.location.href += '#'; + }); + $rootScope.$digest(); + + expect(fakeWindow.location.href).toBe('http://server/#'); + expect($location.absUrl()).toBe('http://server/'); + + expect(spy.calls.count()).toBe(0); + expect(spy).not.toHaveBeenCalled(); + }); + }); }); describe('integration test with $rootScope', function() { diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 58d4d013d968..e5719ae84dc0 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -693,10 +693,10 @@ describe('$location', function() { describe('location watch', function() { - it('should not update browser if only the empty hash fragment is cleared by updating the search', function() { + it('should not update browser if only the empty hash fragment is cleared', function() { initService({supportHistory: true}); - mockUpBrowser({initialUrl:'http://new.com/a/b#', baseHref:'/base/'}); - inject(function($rootScope, $browser, $location) { + mockUpBrowser({initialUrl: 'http://new.com/a/b#', baseHref: '/base/'}); + inject(function($browser, $rootScope) { $browser.url('http://new.com/a/b'); var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').and.callThrough(); $rootScope.$digest(); @@ -707,10 +707,11 @@ describe('$location', function() { it('should not replace browser url if only the empty hash fragment is cleared', function() { initService({html5Mode: true, supportHistory: true}); - mockUpBrowser({initialUrl:'http://new.com/#', baseHref: '/'}); - inject(function($browser, $location) { - expect($browser.url()).toBe('http://new.com/#'); + mockUpBrowser({initialUrl: 'http://new.com/#', baseHref: '/'}); + inject(function($browser, $location, $window) { + expect($browser.url()).toBe('http://new.com/'); expect($location.absUrl()).toBe('http://new.com/'); + expect($window.location.href).toBe('http://new.com/#'); }); });