From a5cd4b2ad27d65fd55d8f5628896a234dfd38a82 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 14:33:29 +0200 Subject: [PATCH 1/7] added condition for area tags too (not only ) --- src/ng/location.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index 9ab99cb719fa..4bfba147e414 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -622,9 +622,10 @@ function $LocationProvider(){ var elm = jqLite(event.target); - // traverse the DOM up to find first A tag - while (lowercase(elm[0].nodeName) !== 'a') { - // ignore rewriting if no A tag (reached root element, or no parent - removed from document) + + // traverse the DOM up to find first A or AREA tag + while (lowercase(elm[0].nodeName) !== 'a' && lowercase(elm[0].nodeName) !== 'area') { + // ignore rewriting if no A or AREA tag (reached root element, or no parent - removed from document) if (elm[0] === $rootElement[0] || !(elm = elm.parent())[0]) return; } From ff75eb60986f8da888087bb4afcaea93ffde5cbd Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 14:39:05 +0200 Subject: [PATCH 2/7] duplicated anchor test to test area elments as well --- test/ng/locationSpec.js | 486 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 485 insertions(+), 1 deletion(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index ff823d306efd..4be7e9eb3b24 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -775,7 +775,491 @@ describe('$location', function() { }); - describe('link rewriting', function() { + describe('link () rewriting', function() { + + var root, link, originalBrowser, lastEventPreventDefault; + + function configureService(linkHref, html5Mode, supportHist, attrs, content) { + 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; + } + + link = jqLite('' + content + '')[0]; + + $provide.value('$sniffer', {history: supportHist}); + $locationProvider.html5Mode(html5Mode); + $locationProvider.hashPrefix('!'); + return function($rootElement, $document) { + $rootElement.append(link); + root = $rootElement[0]; + // we need to do this otherwise we can't simulate events + $document.find('body').append($rootElement); + }; + }); + } + + function initBrowser() { + return function($browser){ + $browser.url('http://host.com/base'); + $browser.$$baseHref = '/base/index.html'; + }; + } + + function initLocation() { + return function($browser, $location, $rootElement) { + originalBrowser = $browser.url(); + // we have to prevent the default operation, as we need to test absolute links (http://...) + // and navigating to these links would kill jstd + $rootElement.on('click', function(e) { + lastEventPreventDefault = e.isDefaultPrevented(); + e.preventDefault(); + }); + }; + } + + function expectRewriteTo($browser, url) { + expect(lastEventPreventDefault).toBe(true); + expect($browser.url()).toBe(url); + } + + function expectNoRewrite($browser) { + expect(lastEventPreventDefault).toBe(false); + expect($browser.url()).toBe(originalBrowser); + } + + afterEach(function() { + dealoc(root); + dealoc(document.body); + }); + + + it('should rewrite rel link to new url when history enabled on new browser', function() { + configureService('link?a#b', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/link?a#b'); + } + ); + }); + + + it('should do nothing if already on the same URL', function() { + configureService('/base/', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/'); + + jqLite(link).attr('href', 'http://host.com/base/foo'); + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/foo'); + + jqLite(link).attr('href', 'http://host.com/base/'); + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/'); + + jqLite(link). + attr('href', 'http://host.com/base/foo'). + on('click', function(e) { e.preventDefault(); }); + browserTrigger(link, 'click'); + expect($browser.url()).toBe('http://host.com/base/'); + } + ); + }); + + + it('should rewrite abs link to new url when history enabled on new browser', function() { + configureService('/base/link?a#b', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/link?a#b'); + } + ); + }); + + + it('should rewrite rel link to hashbang url when history enabled on old browser', function() { + configureService('link?a#b', true, false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/index.html#!/link?a#b'); + } + ); + }); + + + it('should rewrite abs link to hashbang url when history enabled on old browser', function() { + configureService('/base/link?a#b', true, false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/index.html#!/link?a#b'); + } + ); + }); + + + it('should not rewrite full url links do different domain', function() { + configureService('http://www.dot.abc/a?b=c', true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite links with target="_blank"', function() { + configureService('/a?b=c', true, true, 'target="_blank"'); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite links with target specified', function() { + configureService('/a?b=c', true, true, 'target="some-frame"'); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should rewrite full url links to same domain and base path', function() { + configureService('http://host.com/base/new', true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectRewriteTo($browser, 'http://host.com/base/index.html#!/new'); + } + ); + }); + + + it('should rewrite when clicked span inside link', function() { + configureService('some/link', true, true, '', 'link'); + inject( + initBrowser(), + initLocation(), + function($browser) { + var span = jqLite(link).find('span'); + + browserTrigger(span, 'click'); + expectRewriteTo($browser, 'http://host.com/base/some/link'); + } + ); + }); + + + it('should not rewrite when link to different base path when history enabled on new browser', + function() { + configureService('/other_base/link', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when link to different base path when history enabled on old browser', + function() { + configureService('/other_base/link', true, false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when link to different base path when history disabled', function() { + configureService('/other_base/link', false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when full link to different base path when history enabled on new browser', + function() { + configureService('http://host.com/other_base/link', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when full link to different base path when history enabled on old browser', + function() { + configureService('http://host.com/other_base/link', true, false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when full link to different base path when history disabled', function() { + configureService('http://host.com/other_base/link', false); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click'); + expectNoRewrite($browser); + } + ); + }); + + + // don't run next tests on IE<9, as browserTrigger does not simulate pressed keys + if (!(msie < 9)) { + + it('should not rewrite when clicked with ctrl pressed', function() { + configureService('/a?b=c', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click', ['ctrl']); + expectNoRewrite($browser); + } + ); + }); + + + it('should not rewrite when clicked with meta pressed', function() { + configureService('/a?b=c', true, true); + inject( + initBrowser(), + initLocation(), + function($browser) { + browserTrigger(link, 'click', ['meta']); + expectNoRewrite($browser); + } + ); + }); + } + + + it('should not mess up hash urls when clicking on links in hashbang mode', function() { + var base; + module(function() { + return function($browser) { + window.location.hash = 'someHash'; + base = window.location.href + $browser.url(base); + base = base.split('#')[0]; + } + }); + inject(function($rootScope, $compile, $browser, $rootElement, $document, $location) { + // we need to do this otherwise we can't simulate events + $document.find('body').append($rootElement); + + var element = $compile('v1v2')($rootScope); + $rootElement.append(element); + var av1 = $rootElement.find('a').eq(0); + var av2 = $rootElement.find('a').eq(1); + + + browserTrigger(av1, 'click'); + expect($browser.url()).toEqual(base + '#/view1'); + + browserTrigger(av2, 'click'); + expect($browser.url()).toEqual(base + '#/view2'); + + $rootElement.remove(); + }); + }); + + + it('should not mess up hash urls when clicking on links in hashbang mode with a prefix', + function() { + var base; + module(function($locationProvider) { + return function($browser) { + window.location.hash = '!someHash'; + $browser.url(base = window.location.href); + base = base.split('#')[0]; + $locationProvider.hashPrefix('!'); + } + }); + inject(function($rootScope, $compile, $browser, $rootElement, $document, $location) { + // we need to do this otherwise we can't simulate events + $document.find('body').append($rootElement); + + var element = $compile('v1v2')($rootScope); + $rootElement.append(element); + var av1 = $rootElement.find('a').eq(0); + var av2 = $rootElement.find('a').eq(1); + + + browserTrigger(av1, 'click'); + expect($browser.url()).toEqual(base + '#!/view1'); + + browserTrigger(av2, 'click'); + expect($browser.url()).toEqual(base + '#!/view2'); + }); + }); + + + it('should not intercept clicks outside the current hash prefix', function() { + var base, clickHandler; + module(function($provide) { + $provide.value('$rootElement', { + on: function(event, handler) { + expect(event).toEqual('click'); + clickHandler = handler; + }, + off: noop + }); + return function($browser) { + $browser.url(base = 'http://server/'); + } + }); + inject(function($location) { + // make IE happy + jqLite(window.document.body).html('link'); + + var event = { + target: jqLite(window.document.body).find('a')[0], + preventDefault: jasmine.createSpy('preventDefault') + }; + + + clickHandler(event); + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + }); + + + it('should not intercept hash link clicks outside the app base url space', function() { + var base, clickHandler; + module(function($provide) { + $provide.value('$rootElement', { + on: function(event, handler) { + expect(event).toEqual('click'); + clickHandler = handler; + }, + off: angular.noop + }); + return function($browser) { + $browser.url(base = 'http://server/'); + } + }); + inject(function($rootScope, $compile, $browser, $rootElement, $document, $location) { + // make IE happy + jqLite(window.document.body).html('link'); + + var event = { + target: jqLite(window.document.body).find('a')[0], + preventDefault: jasmine.createSpy('preventDefault') + }; + + + clickHandler(event); + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + }); + + + // regression https://github.com/angular/angular.js/issues/1058 + it('should not throw if element was removed', inject(function($document, $rootElement, $location) { + // we need to do this otherwise we can't simulate events + $document.find('body').append($rootElement); + + $rootElement.html(''); + var button = $rootElement.find('button'); + + button.on('click', function() { + button.remove(); + }); + browserTrigger(button, 'click'); + })); + + + it('should not throw when clicking an SVGAElement link', function() { + var base; + module(function($locationProvider) { + return function($browser) { + window.location.hash = '!someHash'; + $browser.url(base = window.location.href); + base = base.split('#')[0]; + $locationProvider.hashPrefix('!'); + } + }); + inject(function($rootScope, $compile, $browser, $rootElement, $document, $location) { + // we need to do this otherwise we can't simulate events + $document.find('body').append($rootElement); + var template = ''; + var element = $compile(template)($rootScope); + + $rootElement.append(element); + var av1 = $rootElement.find('a').eq(0); + expect(function() { + browserTrigger(av1, 'click'); + }).not.toThrow(); + }); + }); + }); + + + describe('link () rewriting', function() { var root, link, originalBrowser, lastEventPreventDefault; From aa870b0f4b1fbcd76b22213a8cf64c26c2b2bef9 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 14:46:35 +0200 Subject: [PATCH 3/7] removed a duplicated test from the newly added ones for the area\n area elements cannot have child nodes, so this test would fail --- test/ng/locationSpec.js | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 4be7e9eb3b24..ba851040e036 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -968,22 +968,7 @@ describe('$location', function() { } ); }); - - - it('should rewrite when clicked span inside link', function() { - configureService('some/link', true, true, '', 'link'); - inject( - initBrowser(), - initLocation(), - function($browser) { - var span = jqLite(link).find('span'); - - browserTrigger(span, 'click'); - expectRewriteTo($browser, 'http://host.com/base/some/link'); - } - ); - }); - + it('should not rewrite when link to different base path when history enabled on new browser', function() { From 8b4880efa43c40a4def0d9571eb42fb01685bbe5 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 15:17:51 +0200 Subject: [PATCH 4/7] added click/dblclick handler for area elements too --- src/ngScenario/dsl.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ngScenario/dsl.js b/src/ngScenario/dsl.js index 50d20c9510c9..bfeb6e30cdc2 100644 --- a/src/ngScenario/dsl.js +++ b/src/ngScenario/dsl.js @@ -377,7 +377,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('click')[0]; - if (href && elements[0].nodeName.toUpperCase() === 'A' && eventProcessDefault) { + if (href && elements[0].nodeName.toUpperCase() === 'A' && elements[0].nodeName.toUpperCase() === 'AREA' && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); @@ -394,7 +394,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('dblclick')[0]; - if (href && elements[0].nodeName.toUpperCase() === 'A' && eventProcessDefault) { + if (href && elements[0].nodeName.toUpperCase() === 'A' && elements[0].nodeName.toUpperCase() === 'AREA' && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); From a1e189e7eccc40cb353b70b62ecb834797041f03 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 15:21:15 +0200 Subject: [PATCH 5/7] added click/dblclick handler for area elements too --- src/ngScenario/dsl.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ngScenario/dsl.js b/src/ngScenario/dsl.js index bfeb6e30cdc2..1d389b50612f 100644 --- a/src/ngScenario/dsl.js +++ b/src/ngScenario/dsl.js @@ -377,7 +377,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('click')[0]; - if (href && elements[0].nodeName.toUpperCase() === 'A' && elements[0].nodeName.toUpperCase() === 'AREA' && eventProcessDefault) { + if (href && (elements[0].nodeName.toUpperCase() === 'A' || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); @@ -394,7 +394,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('dblclick')[0]; - if (href && elements[0].nodeName.toUpperCase() === 'A' && elements[0].nodeName.toUpperCase() === 'AREA' && eventProcessDefault) { + if (href && (elements[0].nodeName.toUpperCase() === 'A' || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); From aba255c68813047ac3bc9f66c20742dd65f8a3c5 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Wed, 22 Jan 2014 15:41:53 +0200 Subject: [PATCH 6/7] rewriten 2 lines of code for jsHint to pass --- src/ngScenario/dsl.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ngScenario/dsl.js b/src/ngScenario/dsl.js index 1d389b50612f..3524ca1828f9 100644 --- a/src/ngScenario/dsl.js +++ b/src/ngScenario/dsl.js @@ -377,7 +377,8 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('click')[0]; - if (href && (elements[0].nodeName.toUpperCase() === 'A' || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { + if (href && (elements[0].nodeName.toUpperCase() === 'A' + || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); @@ -394,7 +395,8 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('dblclick')[0]; - if (href && (elements[0].nodeName.toUpperCase() === 'A' || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { + if (href && (elements[0].nodeName.toUpperCase() === 'A' + || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); From 8db690e78e9a890d3e7d9fdc577983c0f1eebed7 Mon Sep 17 00:00:00 2001 From: Bogdan Gradinariu Date: Thu, 23 Jan 2014 09:53:41 +0200 Subject: [PATCH 7/7] revert ngScenario modifications since it is deprecated (as discussed here: https://github.com/angular/angular.js/pull/5933#issuecomment-33085663) --- src/ngScenario/dsl.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ngScenario/dsl.js b/src/ngScenario/dsl.js index 3524ca1828f9..50d20c9510c9 100644 --- a/src/ngScenario/dsl.js +++ b/src/ngScenario/dsl.js @@ -377,8 +377,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('click')[0]; - if (href && (elements[0].nodeName.toUpperCase() === 'A' - || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { + if (href && elements[0].nodeName.toUpperCase() === 'A' && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done); @@ -395,8 +394,7 @@ angular.scenario.dsl('element', function() { var href = elements.attr('href'); var eventProcessDefault = elements.trigger('dblclick')[0]; - if (href && (elements[0].nodeName.toUpperCase() === 'A' - || elements[0].nodeName.toUpperCase() === 'AREA') && eventProcessDefault) { + if (href && elements[0].nodeName.toUpperCase() === 'A' && eventProcessDefault) { this.application.navigateTo(href, function() { done(); }, done);