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

feat($location): test $location enhancements from #6421 and fix them #6899

Closed
wants to merge 3 commits into from
Closed

feat($location): test $location enhancements from #6421 and fix them #6899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4e6176
Potential fix for #6162
Feb 23, 2014
712dcd0
Parse anchors in IE9
Mar 16, 2014
32acecd
feat($location): test $location enhancements from #6421 and fix them
caitp Mar 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/ng/location.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,16 @@ function LocationHashbangInHtml5Url(appBase, hashPrefix) {
return appBaseNoFile;
}
};

this.$$compose = function() {
var search = toKeyValue(this.$$search),
hash = this.$$hash ? '#' + encodeUriSegment(this.$$hash) : '';

this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
// include hashPrefix in $$absUrl when $$url is empty so IE8 & 9 do not reload page because of removal of '#'
this.$$absUrl = appBase + hashPrefix + this.$$url;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we cause the page reload here? when an app uses routing, the path is always set, so this override shouldn't change anything.

if the path is not set then we should investigate why.

the original thinking was that we don't want to append '#' in hashbang mode unless you actually use $location, otherwise you end up with apps that change the url needlessly, which is something we wanted to avoid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was added by the original PR, so I think we have to ask them. I don't have access to IE9 to debug that easily except on SL, so it's hard for me to say.

};

}


Expand Down Expand Up @@ -642,6 +652,39 @@ function $LocationProvider(){
absHref = urlResolve(absHref.animVal).href;
}

// Make relative links work in HTML5 mode for legacy browsers (or at least IE8 & 9)
// The href should be a regular url e.g. /link/somewhere or link/somewhere or ../somewhere or
// somewhere#anchor or http://example.com/somewhere
if (LocationMode === LocationHashbangInHtml5Url) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it would make sense to have a "preprocessing" method in LocationMode, so that this code could be removed from here, and more easily extended for the other location modes. I guess it's sort of like a special version of $$rewrite that wants the relative url rather than the absolute

// get the actual href attribute - see
// http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
var href = elm.attr('href') || elm.attr('xlink:href');

if (href.indexOf('://') < 0) { // Ignore absolute URLs
var prefix = '#' + hashPrefix;
if (href[0] == '/') {
// absolute path - replace old path
absHref = appBase + prefix + href;
} else if (href[0] == '#') {
// local anchor
absHref = appBase + prefix + ($location.path() || '/') + href;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe http://foo.com/bar#!/#baz is not the right URL for this, that might be cleaner.

} else {
// relative path - join with current path
var stack = $location.path().split("/"),
parts = href.split("/");
for (var i=0; i<parts.length; i++) {
if (parts[i] == ".")
continue;
else if (parts[i] == "..")
stack.pop();
else if (parts[i].length)
stack.push(parts[i]);
}
absHref = appBase + prefix + stack.join('/');
}
}
}

var rewrittenUrl = $location.$$rewrite(absHref);

if (absHref && !elm.attr('target') && rewrittenUrl && !event.isDefaultPrevented()) {
Expand Down
62 changes: 57 additions & 5 deletions test/ng/locationSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,15 +779,22 @@ describe('$location', function() {

var root, link, originalBrowser, lastEventPreventDefault;

function configureService(linkHref, html5Mode, supportHist, attrs, content) {
function configureService(linkHref, html5Mode, supportHist, relLink, attrs, content) {
if (typeof relLink !== "boolean") {
content = attrs;
attrs = relLink;
relLink = false;
}
module(function($provide, $locationProvider) {
attrs = attrs ? ' ' + attrs + ' ' : '';

// fake the base behavior
if (linkHref[0] == '/') {
linkHref = 'http://host.com' + linkHref;
} else if(!linkHref.match(/:\/\//)) {
linkHref = 'http://host.com/base/' + linkHref;
if (!relLink) {
if (linkHref[0] == '/') {
linkHref = 'http://host.com' + linkHref;
} else if(!linkHref.match(/:\/\//)) {
linkHref = 'http://host.com/base/' + linkHref;
}
}

link = jqLite('<a href="' + linkHref + '"' + attrs + '>' + content + '</a>')[0];
Expand Down Expand Up @@ -1067,6 +1074,51 @@ describe('$location', function() {
});


it('should rewrite relative links relative to current path when history disabled', function() {
configureService('link', true, false, true);
inject(
initBrowser(),
initLocation(),
function($browser, $location) {
$location.path('/some');
browserTrigger(link, 'click');
expectRewriteTo($browser, 'http://host.com/base/index.html#!/some/link');
}
);
});


it('should replace current path when link begins with "/" and history disabled', function() {
configureService('/link', true, false, true);
inject(
initBrowser(),
initLocation(),
function($browser, $location) {
$location.path('/some');
browserTrigger(link, 'click');
expectRewriteTo($browser, 'http://host.com/base/index.html#!/link');
}
);
});


it('should replace current hash fragment when link begins with "#" history disabled', function() {
configureService('#link', true, false, true);
inject(
initBrowser(),
initLocation(),
function($browser, $location) {
// Initialize browser URL
$location.path('/some');
$location.hash('foo');
browserTrigger(link, 'click');
expect($location.hash()).toBe('link');
expectRewriteTo($browser, 'http://host.com/base/index.html#!/some#link');
}
);
});


// don't run next tests on IE<9, as browserTrigger does not simulate pressed keys
if (!(msie < 9)) {

Expand Down