From 6d7e7fdea6c3d6551ff40c150aa42e1375d2cb5f Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 11 Apr 2012 23:46:23 -0700 Subject: [PATCH] fix($location): properly rewrite urls in html5 mode with base url set 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 --- docs/content/cookbook/deeplinking.ngdoc | 6 ++-- .../guide/dev_guide.services.$location.ngdoc | 35 ++++++++++--------- src/ng/directive/booleanAttrs.js | 14 ++++---- src/ng/location.js | 22 +++++------- test/ng/locationSpec.js | 21 +++++------ 5 files changed, 45 insertions(+), 53 deletions(-) diff --git a/docs/content/cookbook/deeplinking.ngdoc b/docs/content/cookbook/deeplinking.ngdoc index 4343ded2240c..f242f82e83e4 100644 --- a/docs/content/cookbook/deeplinking.ngdoc +++ b/docs/content/cookbook/deeplinking.ngdoc @@ -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 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 @@ -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: -* ./examples/settings.html -* ./examples/welcome.html +* ./examples/settings.html +* ./examples/welcome.html diff --git a/docs/content/guide/dev_guide.services.$location.ngdoc b/docs/content/guide/dev_guide.services.$location.ngdoc index c99d26b00627..f7ca5dbee739 100644 --- a/docs/content/guide/dev_guide.services.$location.ngdoc +++ b/docs/content/guide/dev_guide.services.$location.ngdoc @@ -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: `<a href="/some?foo=bar">link</a>` +have to do is specify regular URL links, such as: `link` When a user clicks on this link, @@ -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
- Example: `link` -- Links that contain `target="_blank"`
- Example: `link` -- Absolute links that go to a different domain
+- Links that contain `target` element
+ Example: `link` +- Absolute links that go to a different domain
Example: `link` +- Links starting with '/' that lead to a different base path when base is defined
+ Example: `link` + ### Server side @@ -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 `` 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 (``) 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 <base href="" /> !** +takes care of all relative link issues. ### Sending links among different browsers @@ -379,9 +382,9 @@ In this examples we use `` $location.path() = {{$location.path()}}
$location.search() = {{$location.search()}}
$location.hash() = {{$location.hash()}}
- /base/first?a=b | - sec/ond?flag#hash | - external + /base/first?a=b | + sec/ond?flag#hash | + external
@@ -393,9 +396,9 @@ In this examples we use `` $location.path() = {{$location.path()}}
$location.search() = {{$location.search()}}
$location.hash() = {{$location.hash()}}
- /base/first?a=b | - sec/ond?flag#hash | - external + /base/first?a=b | + sec/ond?flag#hash | + external
@@ -445,7 +448,7 @@ In this examples we use `` $compileProvider.directive('ngAddressBar', function() { return function(scope, elm, attrs) { var browser = browsers[attrs.browser], - input = angular.element('').val(browser.url()), + input = angular.element('').val(browser.url()), delay; input.bind('keypress keyup keydown', function() { diff --git a/src/ng/directive/booleanAttrs.js b/src/ng/directive/booleanAttrs.js index e25b259c7908..5da98ef425b0 100644 --- a/src/ng/directive/booleanAttrs.js +++ b/src/ng/directive/booleanAttrs.js @@ -6,7 +6,7 @@ * @restrict A * * @description - * Using 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. @@ -32,10 +32,10 @@
link 1 (link, don't reload)
link 2 (link, don't reload)
- link 3 (link, reload!)
+ link 3 (link, reload!)
anchor (link, don't reload)
anchor (no link)
- link (link, change hash) + link (link, change location) it('should execute ng-click but not reload when href without value', function() { @@ -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'); }); diff --git a/src/ng/location.js b/src/ng/location.js index 9feee5ad87c5..575feb7e1db5 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -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 diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 98167b4e47a7..648fa42da5d4 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -672,6 +672,14 @@ describe('$location', function() { module(function($provide, $locationProvider) { var jqRoot = jqLite('
'); 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('' + content + '')[0]; root = jqRoot.append(link)[0]; @@ -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(