Skip to content

Commit

Permalink
Merge pull request #13520 from jasonmit/location-none-fail-tests
Browse files Browse the repository at this point in the history
[Bugfix] fixes bugs around baseURL and rootURL in HistoryLocation and introduces rootURL to NoneLocation
  • Loading branch information
rwjblue committed May 20, 2016
2 parents 65dc1e8 + 6312609 commit 9a993e5
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 10 deletions.
7 changes: 5 additions & 2 deletions packages/ember-application/lib/system/application-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,13 @@ const ApplicationInstance = EngineInstance.extend({
}
};

let location = get(router, 'location');

// Keeps the location adapter's internal URL in-sync
get(router, 'location').setURL(url);
location.setURL(url);

return router.handleURL(url).then(handleResolve, handleReject);
// getURL returns the set url with the rootURL stripped off
return router.handleURL(location.getURL()).then(handleResolve, handleReject);
}
});

Expand Down
6 changes: 4 additions & 2 deletions packages/ember-routing/lib/location/history_location.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ export default EmberObject.extend({
rootURL = rootURL.replace(/\/$/, '');
baseURL = baseURL.replace(/\/$/, '');

// remove baseURL and rootURL from path
var url = path.replace(baseURL, '').replace(rootURL, '');
// remove baseURL and rootURL from start of path
var url = path
.replace(new RegExp('^' + baseURL), '')
.replace(new RegExp('^' + rootURL), '');

var search = location.search || '';
url += search;
Expand Down
40 changes: 34 additions & 6 deletions packages/ember-routing/lib/location/none_location.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { assert } from 'ember-metal/debug';
import { get } from 'ember-metal/property_get';
import { set } from 'ember-metal/property_set';
import EmberObject from 'ember-runtime/system/object';
Expand All @@ -22,15 +23,38 @@ export default EmberObject.extend({
implementation: 'none',
path: '',

detect() {
let rootURL = this.rootURL;

assert('rootURL must end with a trailing forward slash e.g. "/app/"',
rootURL.charAt(rootURL.length - 1) === '/');
},

/**
Will be pre-pended to path.
@private
@property rootURL
@default '/'
*/
rootURL: '/',

/**
Returns the current path.
Returns the current path without `rootURL`.
@private
@method getURL
@return {String} path
*/
getURL() {
return get(this, 'path');
let path = get(this, 'path');
let rootURL = get(this, 'rootURL');

// remove trailing slashes if they exists
rootURL = rootURL.replace(/\/$/, '');

// remove rootURL from url
return path.replace(new RegExp('^' + rootURL), '');
},

/**
Expand Down Expand Up @@ -83,9 +107,13 @@ export default EmberObject.extend({
@return {String} url
*/
formatURL(url) {
// The return value is not overly meaningful, but we do not want to throw
// errors when test code renders templates containing {{action href=true}}
// helpers.
return url;
let rootURL = get(this, 'rootURL');

if (url !== '') {
// remove trailing slashes if they exists
rootURL = rootURL.replace(/\/$/, '');
}

return rootURL + url;
}
});
34 changes: 34 additions & 0 deletions packages/ember-routing/tests/location/history_location_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,40 @@ QUnit.test('HistoryLocation.getURL() returns the current url, excluding both roo
equal(location.getURL(), '/foo/bar');
});

QUnit.test('HistoryLocation.getURL() returns the current url, does not remove rootURL if its not at start of url', function() {
expect(1);

HistoryTestLocation.reopen({
init() {
this._super(...arguments);

set(this, 'location', mockBrowserLocation('/foo/bar/baz'));
set(this, 'rootURL', '/bar/');
}
});

createLocation();

equal(location.getURL(), '/foo/bar/baz');
});

QUnit.test('HistoryLocation.getURL() returns the current url, does not remove baseURL if its not at start of url', function() {
expect(1);

HistoryTestLocation.reopen({
init() {
this._super(...arguments);

set(this, 'location', mockBrowserLocation('/foo/bar/baz'));
set(this, 'baseURL', '/bar/');
}
});

createLocation();

equal(location.getURL(), '/foo/bar/baz');
});

QUnit.test('HistoryLocation.getURL() includes location.search', function() {
expect(1);

Expand Down
69 changes: 69 additions & 0 deletions packages/ember-routing/tests/location/none_location_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { set } from 'ember-metal/property_set';
import run from 'ember-metal/run_loop';
import NoneLocation from 'ember-routing/location/none_location';

var NoneTestLocation, location;

function createLocation(options) {
if (!options) { options = {}; }
location = NoneTestLocation.create(options);
}

QUnit.module('Ember.NoneLocation', {
setup() {
NoneTestLocation = NoneLocation.extend({});
},

teardown() {
run(function() {
if (location) { location.destroy(); }
});
}
});

QUnit.test('NoneLocation.formatURL() returns the current url always appending rootURL', function() {
expect(1);

NoneTestLocation.reopen({
init() {
this._super(...arguments);
set(this, 'rootURL', '/en/');
}
});

createLocation();

equal(location.formatURL('/foo/bar'), '/en/foo/bar');
});

QUnit.test('NoneLocation.getURL() returns the current path minus rootURL', function() {
expect(1);

NoneTestLocation.reopen({
init() {
this._super(...arguments);
set(this, 'rootURL', '/foo/');
set(this, 'path', '/foo/bar');
}
});

createLocation();

equal(location.getURL(), '/bar');
});

QUnit.test('NonoLocation.getURL() will remove the rootURL only from the beginning of a url', function() {
expect(1);

NoneTestLocation.reopen({
init() {
this._super(...arguments);
set(this, 'rootURL', '/bar/');
set(this, 'path', '/foo/bar/baz');
}
});

createLocation();

equal(location.getURL(), '/foo/bar/baz');
});

0 comments on commit 9a993e5

Please sign in to comment.