From 408901e3b8a5e518fb94cc3a18a6287a407c4beb Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 11 Nov 2014 17:10:29 +0000 Subject: [PATCH 1/2] fix($browser): prevent infinite digests when clearing the hash of a url By using `location.hash` to update the current browser location when only the hash has changed, we prevent the browser from attempting to reload. Closes #9629 Closes #9635 Closes #10228 --- src/ng/browser.js | 9 ++++++++- test/ng/browserSpecs.js | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index a9299343778d..af2dd70e76a9 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -61,6 +61,11 @@ function Browser(window, document, $log, $sniffer) { } } + function getHash(url) { + var index = url.indexOf('#'); + return index === -1 ? '' : url.substr(index + 1); + } + /** * @private * Note: this method is used only by scenario runner @@ -190,8 +195,10 @@ function Browser(window, document, $log, $sniffer) { } if (replace) { location.replace(url); - } else { + } else if (!sameBase) { location.href = url; + } else { + location.hash = getHash(url); } } return self; diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index 3778e0a6d904..d665bbdc87a0 100755 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -1,5 +1,7 @@ 'use strict'; +/* global getHash:true, stripHash:true */ + var historyEntriesLength; var sniffer = {}; @@ -51,6 +53,12 @@ function MockWindow(options) { mockWindow.history.state = null; historyEntriesLength++; }, + get hash() { + return getHash(locationHref); + }, + set hash(value) { + locationHref = stripHash(locationHref) + '#' + value; + }, replace: function(url) { locationHref = url; mockWindow.history.state = null; @@ -550,6 +558,17 @@ describe('browser', function() { expect(locationReplace).not.toHaveBeenCalled(); }); + it("should retain the # character when the only change is clearing the hash fragment, to prevent page reload", function() { + sniffer.history = true; + + browser.url('http://server/#123'); + expect(fakeWindow.location.href).toEqual('http://server/#123'); + + browser.url('http://server/'); + expect(fakeWindow.location.href).toEqual('http://server/#'); + + }); + it('should use location.replace when history.replaceState not available', function() { sniffer.history = false; browser.url('http://new.org', true); @@ -561,6 +580,7 @@ describe('browser', function() { expect(fakeWindow.location.href).toEqual('http://server/'); }); + it('should use location.replace and not use replaceState when the url only changed in the hash fragment to please IE10/11', function() { sniffer.history = true; browser.url('http://server/#123', true); @@ -572,6 +592,7 @@ describe('browser', function() { expect(fakeWindow.location.href).toEqual('http://server/'); }); + it('should return $browser to allow chaining', function() { expect(browser.url('http://any.com')).toBe(browser); }); From e056f853288121a91130852eff319d970ca14122 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 3 Dec 2014 10:43:21 +0000 Subject: [PATCH 2/2] fix($location): allow hash fragments with hashPrefix in hash-bang location urls Previously if there was a hash fragment but no hashPrefix we would throw an error. Now we assume that the hash-bang path is empty and that the hash is a valid fragment. This prevents unnecessary exceptions where we clear the hashBang path, say by navigating back to the base url, where the $browser leaves an empty hash symbol on the URL to ensure there is no browser reload. BREAKING CHANGE: We no longer throw an `ihshprfx` error if the URL after the base path contains only a hash fragment. Previously, if the base URL was `http://abc.com/base/` and the hashPrefix is `!` then trying to parse `http://abc.com/base/#some-fragment` would have thrown an error. Now we simply assume it is a normal fragment and that the path is empty, resulting `$location.absUrl() === "http://abc.com/base/#!/#some-fragment"`. This should not break any applications, but you can no longer rely on receiving the `ihshprfx` error for paths that have the syntax above. It is actually more similar to what currently happens for invalid extra paths anyway: If the base URL and hashPrfix are set up as above, then `http://abc.com/base/other/path` does not throw an error but just ignores the extra path: `http://abc.com/base`. Closes #9629 Closes #9635 Closes #10228 --- src/ng/location.js | 27 ++++++++++++++++++--------- test/ng/locationSpec.js | 22 ++++++++++++++++++---- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index 4338bc0af03d..86361abe8952 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -183,16 +183,25 @@ function LocationHashbangUrl(appBase, hashPrefix) { */ this.$$parse = function(url) { var withoutBaseUrl = beginsWith(appBase, url) || beginsWith(appBaseNoFile, url); - var withoutHashUrl = withoutBaseUrl.charAt(0) == '#' - ? beginsWith(hashPrefix, withoutBaseUrl) - : (this.$$html5) - ? withoutBaseUrl - : ''; - - if (!isString(withoutHashUrl)) { - throw $locationMinErr('ihshprfx', 'Invalid url "{0}", missing hash prefix "{1}".', url, - hashPrefix); + var withoutHashUrl; + + if (withoutBaseUrl.charAt(0) === '#') { + + // The rest of the url starts with a hash so we have + // got either a hashbang path or a plain hash fragment + withoutHashUrl = beginsWith(hashPrefix, withoutBaseUrl); + if (isUndefined(withoutHashUrl)) { + // There was no hashbang prefix so we just have a hash fragment + withoutHashUrl = withoutBaseUrl; + } + + } else { + // There was no hashbang path nor hash fragment: + // If we are in HTML5 mode we use what is left as the path; + // Otherwise we ignore what is left + withoutHashUrl = this.$$html5 ? withoutBaseUrl : ''; } + parseAppUrl(withoutHashUrl, this); this.$$path = removeWindowsDriveName(this.$$path, withoutHashUrl, appBase); diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index f99dd3997523..1ba121900546 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -554,10 +554,24 @@ describe('$location', function() { }); - it('should throw error when invalid hashbang prefix given', function() { - expect(function() { - url.$$parse('http://www.server.org:1234/base#/path'); - }).toThrowMinErr('$location', 'ihshprfx', 'Invalid url "http://www.server.org:1234/base#/path", missing hash prefix "#!".'); + it('should insert default hashbang if a hash is given with no hashbang prefix', function() { + + url.$$parse('http://www.server.org:1234/base#/path'); + expect(url.absUrl()).toBe('http://www.server.org:1234/base#!#%2Fpath'); + expect(url.hash()).toBe('/path'); + expect(url.path()).toBe(''); + + url.$$parse('http://www.server.org:1234/base#'); + expect(url.absUrl()).toBe('http://www.server.org:1234/base'); + expect(url.hash()).toBe(''); + expect(url.path()).toBe(''); + }); + + it('should ignore extra path segments if no hashbang is given', function() { + url.$$parse('http://www.server.org:1234/base/extra/path'); + expect(url.absUrl()).toBe('http://www.server.org:1234/base'); + expect(url.path()).toBe(''); + expect(url.hash()).toBe(''); });