Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($location): don't initialize hash url unnecessarily
Browse files Browse the repository at this point in the history
After a recent refactoring using $location in the default hashbang mode would result
in hash url being initialized unnecessarily in cases when the base url didn't end
with a slash.

for example http://localhost:8000/temp.html would get rewritten as
http://location:8000/temp.html#/temp.html by error.
  • Loading branch information
IgorMinar committed Aug 12, 2013
1 parent 04cebcc commit d4d34ab
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 8 deletions.
22 changes: 19 additions & 3 deletions src/ng/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ function serverBase(url) {
* @param {string} basePrefix url path prefix
*/
function LocationHtml5Url(appBase, basePrefix) {
this.$$html5 = true;
basePrefix = basePrefix || '';
var appBaseNoFile = stripFile(appBase);
/**
Expand Down Expand Up @@ -140,7 +141,8 @@ function LocationHtml5Url(appBase, basePrefix) {

/**
* LocationHashbangUrl represents url
* This object is exposed as $location service when html5 history api is disabled or not supported
* This object is exposed as $location service when developer doesn't opt into html5 mode.
* It also serves as the base class for html5 mode fallback on legacy browsers.
*
* @constructor
* @param {string} appBase application base URL
Expand All @@ -149,18 +151,25 @@ function LocationHtml5Url(appBase, basePrefix) {
function LocationHashbangUrl(appBase, hashPrefix) {
var appBaseNoFile = stripFile(appBase);

matchUrl(appBase, this);


/**
* Parse given hashbang url into properties
* @param {string} url Hashbang url
* @private
*/
this.$$parse = function(url) {
matchUrl(url, this);
var withoutBaseUrl = beginsWith(appBase, url) || beginsWith(appBaseNoFile, url);
if (!isString(withoutBaseUrl)) {
throw $locationMinErr('istart', 'Invalid url "{0}", does not start with "{1}".', url, appBase);
}
var withoutHashUrl = withoutBaseUrl.charAt(0) == '#' ? beginsWith(hashPrefix, withoutBaseUrl) : withoutBaseUrl;
var withoutHashUrl = withoutBaseUrl.charAt(0) == '#'
? beginsWith(hashPrefix, withoutBaseUrl)
: (this.$$html5)
? withoutBaseUrl
: '';

if (!isString(withoutHashUrl)) {
throw $locationMinErr('nohash', 'Invalid url "{0}", missing hash prefix "{1}".', url, hashPrefix);
}
Expand Down Expand Up @@ -198,6 +207,7 @@ function LocationHashbangUrl(appBase, hashPrefix) {
* @param {string} hashPrefix hashbang prefix
*/
function LocationHashbangInHtml5Url(appBase, hashPrefix) {
this.$$html5 = true;
LocationHashbangUrl.apply(this, arguments);

var appBaseNoFile = stripFile(appBase);
Expand All @@ -220,6 +230,12 @@ LocationHashbangInHtml5Url.prototype =
LocationHashbangUrl.prototype =
LocationHtml5Url.prototype = {

/**
* Are we in html5 mode?
* @private
*/
$$html5: false,

/**
* Has any change been replacing ?
* @private
Expand Down
33 changes: 28 additions & 5 deletions test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ describe('$location', function() {
);
});

it('should correctly convert html5 url with path matching basepath to hashbang url', function () {
it('should correctly convert html5 url with path matching basepath to hashbang url', function () {
initService(true, '!', false);
inject(
initBrowser('http://domain.com/base/index.html', '/base/index.html'),
Expand Down Expand Up @@ -1422,16 +1422,39 @@ describe('$location', function() {
describe('LocationHashbangUrl', function() {
var location;

beforeEach(function() {
location = new LocationHashbangUrl('http://server/pre/', 'http://server/pre/#/path');
});

it('should rewrite URL', function() {
location = new LocationHashbangUrl('http://server/pre/', '#');

expect(location.$$rewrite('http://other')).toEqual(undefined);
expect(location.$$rewrite('http://server/pre/')).toEqual('http://server/pre/');
expect(location.$$rewrite('http://server/pre/#otherPath')).toEqual('http://server/pre/#otherPath');
expect(location.$$rewrite('javascript:void(0)')).toEqual(undefined);
});

it("should not set hash if one was not originally specified", function() {
location = new LocationHashbangUrl('http://server/pre/index.html', '#');

location.$$parse('http://server/pre/index.html')
expect(location.url()).toBe('');
expect(location.absUrl()).toBe('http://server/pre/index.html');
});

it("should parse hash if one was specified", function() {
location = new LocationHashbangUrl('http://server/pre/index.html', '#');

location.$$parse('http://server/pre/index.html#/foo/bar')
expect(location.url()).toBe('/foo/bar');
expect(location.absUrl()).toBe('http://server/pre/index.html#/foo/bar');
});


it("should prefix hash url with / if one was originally missing", function() {
location = new LocationHashbangUrl('http://server/pre/index.html', '#');

location.$$parse('http://server/pre/index.html#not-starting-with-slash')
expect(location.url()).toBe('/not-starting-with-slash');
expect(location.absUrl()).toBe('http://server/pre/index.html#/not-starting-with-slash');
});
});


Expand Down

0 comments on commit d4d34ab

Please sign in to comment.