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

Commit

Permalink
fix($location): properly rewrite urls in html5 mode with base url set
Browse files Browse the repository at this point in the history
previously we were doing all kinds of checks to see if we should rewrite the url or not and we
were missing many scenarios. not any more.

with this change, we rewrite the url unless:
- the href is not set
- link has target attribute
- the absolute url of the link doesn't match the absolute prefix for all urls in our app

This also means that ng-ext-link attribute which we previously used to distinguish external
links from app links is not necessary any more. apps can just set target=_self to prevent
rewriting.

BREAKING CHANGE: ng-ext-link directive was removed because it's unnecessary

apps that relied on ng-ext-link should simply replace it with target=_self
  • Loading branch information
IgorMinar committed Apr 12, 2012
1 parent df72852 commit 6d7e7fd
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 53 deletions.
6 changes: 3 additions & 3 deletions docs/content/cookbook/deeplinking.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Deep linking allows you to encode the state of the application in the URL so that it can be
bookmarked and the application can be restored from the URL to the same state.

While <angular/> does not force you to deal with bookmarks in any particular way, it has services
While angular does not force you to deal with bookmarks in any particular way, it has services
which make the common case described here very easy to implement.

# Assumptions
Expand Down Expand Up @@ -33,8 +33,8 @@ In this example we have a simple app which consist of two screens:

The two partials are defined in the following URLs:

* <a href="./examples/settings.html" ng-ext-link>./examples/settings.html</a>
* <a href="./examples/welcome.html" ng-ext-link>./examples/welcome.html</a>
* <a href="examples/settings.html" target="_self">./examples/settings.html</a>
* <a href="examples/welcome.html" target="_self">./examples/welcome.html</a>

<doc:example module="deepLinking">
<doc:source jsfiddle="false">
Expand Down
35 changes: 19 additions & 16 deletions docs/content/guide/dev_guide.services.$location.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ history API or not; the `$location` service makes this transparent to you.
### Html link rewriting

When you use the history API mode, you will need different links in different browser, but all you
have to do is specify regular URL links, such as: `&lt;a href="/some?foo=bar"&gt;link&lt;/a&gt;`
have to do is specify regular URL links, such as: `<a href="/some?foo=bar">link</a>`

When a user clicks on this link,

Expand All @@ -316,12 +316,13 @@ When a user clicks on this link,
In cases like the following, links are not rewritten; instead, the browser will perform a full page
reload to the original link.

- Links with an `ngExtLink` directive<br />
Example: `<a href="/ext/link?a=b" ng-ext-link>link</a>`
- Links that contain `target="_blank"`<br />
Example: `<a href="/ext/link?a=b" target="_blank">link</a>`
- Absolute links that go to a different domain<br />
- Links that contain `target` element<br>
Example: `<a href="/ext/link?a=b" target="_self">link</a>`
- Absolute links that go to a different domain<br>
Example: `<a href="http://angularjs.org/">link</a>`
- Links starting with '/' that lead to a different base path when base is defined<br>
Example: `<a href="/not-my-base/link">link</a>`


### Server side

Expand All @@ -341,11 +342,13 @@ Applications Crawlable}.

### Relative links

Be sure to check all relative links, images, scripts etc. You must use an absolute path because the
path is going to be rewritten. You can use `<base href="" />` tag as well.
Be sure to check all relative links, images, scripts etc. You must either specify the url base in
the head of your main html file (`<base href="/my-base">`) or you must use absolute urls
(starting with `/`) everywhere because relative urls will be resolved to absolute urls using the
initial absolute url of the document, which is often different from the root of the application.

Running Angular apps with the History API enabled from document root is strongly encouraged as it
takes care of all relative link issues. **Otherwise you have to specify &lt;base href="" /&gt; !**
takes care of all relative link issues.

### Sending links among different browsers

Expand Down Expand Up @@ -379,9 +382,9 @@ In this examples we use `<base href="/base/index.html" />`
$location.path() = {{$location.path()}}<br>
$location.search() = {{$location.search()}}<br>
$location.hash() = {{$location.hash()}}<br>
<a href="/base/first?a=b">/base/first?a=b</a> |
<a href="sec/ond?flag#hash">sec/ond?flag#hash</a> |
<a href="/base/another?search" ng-ext-link>external</a>
<a href="http://www.host.com/base/first?a=b">/base/first?a=b</a> |
<a href="http://www.host.com/base/sec/ond?flag#hash">sec/ond?flag#hash</a> |
<a href="/other-base/another?search">external</a>
</div>

<div id="hashbang-mode" ng-controller="HashbangCntl">
Expand All @@ -393,9 +396,9 @@ In this examples we use `<base href="/base/index.html" />`
$location.path() = {{$location.path()}}<br>
$location.search() = {{$location.search()}}<br>
$location.hash() = {{$location.hash()}}<br>
<a href="/base/first?a=b">/base/first?a=b</a> |
<a href="sec/ond?flag#hash">sec/ond?flag#hash</a> |
<a href="/base/another?search" ng-ext-link>external</a>
<a href="http://www.host.com/base/first?a=b">/base/first?a=b</a> |
<a href="http://www.host.com/base/sec/ond?flag#hash">sec/ond?flag#hash</a> |
<a href="/other-base/another?search">external</a>
</div>
</div>

Expand Down Expand Up @@ -445,7 +448,7 @@ In this examples we use `<base href="/base/index.html" />`
$compileProvider.directive('ngAddressBar', function() {
return function(scope, elm, attrs) {
var browser = browsers[attrs.browser],
input = angular.element('<input type="text" />').val(browser.url()),
input = angular.element('<input type="text">').val(browser.url()),
delay;

input.bind('keypress keyup keydown', function() {
Expand Down
14 changes: 7 additions & 7 deletions src/ng/directive/booleanAttrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* @restrict A
*
* @description
* Using <angular/> markup like {{hash}} in an href attribute makes
* Using Angular markup like {{hash}} in an href attribute makes
* the page open to a wrong URL, if the user clicks that link before
* angular has a chance to replace the {{hash}} with actual URL, the
* link will be broken and will most likely return a 404 error.
Expand All @@ -32,10 +32,10 @@
<input ng-model="value" /><br />
<a id="link-1" href ng-click="value = 1">link 1</a> (link, don't reload)<br />
<a id="link-2" href="" ng-click="value = 2">link 2</a> (link, don't reload)<br />
<a id="link-3" ng-href="/{{'123'}}" ng-ext-link>link 3</a> (link, reload!)<br />
<a id="link-3" ng-href="/{{'123'}}">link 3</a> (link, reload!)<br />
<a id="link-4" href="" name="xx" ng-click="value = 4">anchor</a> (link, don't reload)<br />
<a id="link-5" name="xxx" ng-click="value = 5">anchor</a> (no link)<br />
<a id="link-6" ng-href="/{{value}}" ng-ext-link>link</a> (link, change hash)
<a id="link-6" ng-href="{{value}}">link</a> (link, change location)
</doc:source>
<doc:scenario>
it('should execute ng-click but not reload when href without value', function() {
Expand All @@ -60,21 +60,21 @@
it('should execute ng-click but not reload when href empty string and name specified', function() {
element('#link-4').click();
expect(input('value').val()).toEqual('4');
expect(element('#link-4').attr('href')).toBe("");
expect(element('#link-4').attr('href')).toBe('');
});
it('should execute ng-click but not reload when no href but name specified', function() {
element('#link-5').click();
expect(input('value').val()).toEqual('5');
expect(element('#link-5').attr('href')).toBe("");
expect(element('#link-5').attr('href')).toBe('');
});
it('should only change url when only ng-href', function() {
input('value').enter('6');
expect(element('#link-6').attr('href')).toBe("/6");
expect(element('#link-6').attr('href')).toBe('6');
element('#link-6').click();
expect(browser().window().path()).toEqual('/6');
expect(browser().location().url()).toEqual('/6');
});
</doc:scenario>
</doc:example>
Expand Down
22 changes: 8 additions & 14 deletions src/ng/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,22 +500,16 @@ function $LocationProvider(){
elm = elm.parent();
}

var href = elm.attr('href');
if (!href || isDefined(elm.attr('ng-ext-link')) || elm.attr('target')) return;
var absHref = elm.prop('href');

// link to different base path
if (href[0] === '/' && href.indexOf(pathPrefix) !== 0) return;

// remove same domain from full url links (IE7 always returns full hrefs)
href = href.replace(absUrlPrefix, '');

// link to different domain (or base path)
if (href.substr(0, 4) == 'http') return;

// remove pathPrefix from absolute links
href = href.indexOf(pathPrefix) === 0 ? href.substr(pathPrefix.length) : href;
if (!absHref ||
elm.attr('target') ||
absHref.indexOf(absUrlPrefix) !== 0) { // link to different domain or base path
return;
}

currentUrl.url(href);
// update location with href without the prefix
currentUrl.url(absHref.substr(absUrlPrefix.length));
$rootScope.$apply();
event.preventDefault();
// hack to work around FF6 bug 684208 when scenario runner clicks on links
Expand Down
21 changes: 8 additions & 13 deletions test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,14 @@ describe('$location', function() {
module(function($provide, $locationProvider) {
var jqRoot = jqLite('<div></div>');
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;
}

link = jqLite('<a href="' + linkHref + '"' + attrs + '>' + content + '</a>')[0];
root = jqRoot.append(link)[0];

Expand Down Expand Up @@ -784,19 +792,6 @@ describe('$location', function() {
});


it('should not rewrite ngExtLink', function() {
configureService('#new', true, true, 'ng-ext-link');
inject(
initBrowser(),
initLocation(),
function($browser) {
browserTrigger(link, 'click');
expectNoRewrite($browser);
}
);
});


it('should not rewrite full url links do different domain', function() {
configureService('http://www.dot.abc/a?b=c', true);
inject(
Expand Down

0 comments on commit 6d7e7fd

Please sign in to comment.