From 24a37709011a414d18fde7eb7216ff97105f0fed Mon Sep 17 00:00:00 2001 From: Jeff Ardilio Date: Wed, 24 Jul 2013 16:49:03 -0400 Subject: [PATCH 1/3] Route continues to be processed event if providing a redirectTo If a route defines a redirectTo value, the current route should stop processing the route and instead wait to process the route of the redirect. Currently, what is happening is that Angular detects a redirectTo value and updates $location, but then continues processing the route that was intended to be redirected. Templates are loaded, resolves are processed, ng-view then updates the view with a new template and instantiates the controller all for a route that should have been redirected. A common use case for assigning a function to the redirectTo is to validation authentication, permission check, or some other logic to determine if the route should continue or redirect else where. In the end, this happens, but in between, unexpected results may occur by updating the view and instantiating controllers for logic that wasn't expected to be executed. http://plnkr.co/edit/8QlA0ouuePH3p35Ntmjy?p=preview --- src/ngRoute/route.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index de0d90160353..8dad67e59914 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -430,6 +430,7 @@ function $RouteProvider(){ $location.url(next.redirectTo(next.pathParams, $location.path(), $location.search())) .replace(); } + return;//exit out and don't process current next value, wait for next location change from redirect } } From 68aad0e1494203e5027a329c4b630cc17715366a Mon Sep 17 00:00:00 2001 From: Jeff Ardilio Date: Wed, 24 Jul 2013 18:17:42 -0400 Subject: [PATCH 2/3] If a route defines a redirectTo value, the current route should stop processing the route and instead wait to process the route of the redirect. Currently, what is happening is that Angular detects a redirectTo value and updates $location, but then continues processing the route that was intended to be redirected. Templates are loaded, resolves are processed, ng-view then updates the view with a new template and instantiates the controller all for a route that should have been redirected. A common use case for assigning a function to the redirectTo is to validation authentication, permission check, or some other logic to determine if the route should continue or redirect else where. In the end, this happens, but in between, unexpected results may occur by updating the view and instantiating controllers for logic that wasn't expected to be executed. http://plnkr.co/edit/8QlA0ouuePH3p35Ntmjy?p=preview Fix is to check for url change post redirect, it url changed, exit out and don't process current route in route.js. Created new unit tests for ngViewSpec.js to validate that controller of redirected route is not executed --- src/ngRoute/route.js | 5 ++++- test/ngRoute/directive/ngViewSpec.js | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 8dad67e59914..6875bb11baaa 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -423,6 +423,7 @@ function $RouteProvider(){ $route.current = next; if (next) { if (next.redirectTo) { + var url = $location.url(); if (isString(next.redirectTo)) { $location.path(interpolate(next.redirectTo, next.params)).search(next.params) .replace(); @@ -430,7 +431,9 @@ function $RouteProvider(){ $location.url(next.redirectTo(next.pathParams, $location.path(), $location.search())) .replace(); } - return;//exit out and don't process current next value, wait for next location change from redirect + if (url !== $location.url()) { + return;//exit out and don't process current next value, wait for next location change from redirect + } } } diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js index 50531c18df58..1db91c24f0a0 100644 --- a/test/ngRoute/directive/ngViewSpec.js +++ b/test/ngRoute/directive/ngViewSpec.js @@ -509,6 +509,28 @@ describe('ngView', function() { }); }); + it('should not instantiate controller of redirected route', function() { + var firstController = jasmine.createSpy('first controller spy'); + var secondController = jasmine.createSpy('second controller spy'); + module(function($routeProvider) { + $routeProvider.when('/redirect', { + template: 'redirect view', + redirectTo: '/redirected', + controller: firstController + }); + $routeProvider.when('/redirected', { + template: 'redirected view', + controller: secondController + }); + }); + inject(function($route, $location, $rootScope) { + $location.path('/redirect'); + $rootScope.$digest(); + expect(firstController).not.toHaveBeenCalled(); + expect(secondController).toHaveBeenCalled(); + }); + }); + describe('ngAnimate ', function() { var window, vendorPrefix; var body, element, $rootElement; From 63c162bebd910a7e89a96cf3047631d7f37725fa Mon Sep 17 00:00:00 2001 From: Jeff Ardilio Date: Wed, 24 Jul 2013 18:17:42 -0400 Subject: [PATCH 3/3] fix($route): prevent processing of redirected route Fixes issue where templateUrl and resolve values are processed and resolved and controllers instantiated for a route which was intented to be redirected. This prevents processing of a route which may have precondition qualifiers to execute and instead follow and process only the redirect path. --- src/ngRoute/route.js | 5 ++++- test/ngRoute/directive/ngViewSpec.js | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 8dad67e59914..6875bb11baaa 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -423,6 +423,7 @@ function $RouteProvider(){ $route.current = next; if (next) { if (next.redirectTo) { + var url = $location.url(); if (isString(next.redirectTo)) { $location.path(interpolate(next.redirectTo, next.params)).search(next.params) .replace(); @@ -430,7 +431,9 @@ function $RouteProvider(){ $location.url(next.redirectTo(next.pathParams, $location.path(), $location.search())) .replace(); } - return;//exit out and don't process current next value, wait for next location change from redirect + if (url !== $location.url()) { + return;//exit out and don't process current next value, wait for next location change from redirect + } } } diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js index 50531c18df58..1db91c24f0a0 100644 --- a/test/ngRoute/directive/ngViewSpec.js +++ b/test/ngRoute/directive/ngViewSpec.js @@ -509,6 +509,28 @@ describe('ngView', function() { }); }); + it('should not instantiate controller of redirected route', function() { + var firstController = jasmine.createSpy('first controller spy'); + var secondController = jasmine.createSpy('second controller spy'); + module(function($routeProvider) { + $routeProvider.when('/redirect', { + template: 'redirect view', + redirectTo: '/redirected', + controller: firstController + }); + $routeProvider.when('/redirected', { + template: 'redirected view', + controller: secondController + }); + }); + inject(function($route, $location, $rootScope) { + $location.path('/redirect'); + $rootScope.$digest(); + expect(firstController).not.toHaveBeenCalled(); + expect(secondController).toHaveBeenCalled(); + }); + }); + describe('ngAnimate ', function() { var window, vendorPrefix; var body, element, $rootElement;