From c6392616ea5245bd0d2f77dded0b948d9e2637c8 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Mon, 14 Jan 2013 20:01:51 +0100 Subject: [PATCH] fix($route): support route params not separated with slashes. Commit 773ac4a broke support for route parameters that are not seperated from other route parts by slashes, which this change fixes. It also adds some documentation about path parameters to the when() method and escapes all regular expression special characters in the URL, not just some. --- src/ng/route.js | 47 ++++++++++++++++++++++++++------------ test/ng/routeParamsSpec.js | 12 ++++++++++ test/ng/routeSpec.js | 32 ++++++++++++++++++-------- 3 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/ng/route.js b/src/ng/route.js index 361b8ac33abc..91e4adb358b9 100644 --- a/src/ng/route.js +++ b/src/ng/route.js @@ -20,8 +20,13 @@ function $RouteProvider(){ * * @param {string} path Route path (matched against `$location.path`). If `$location.path` * contains redundant trailing slash or is missing one, the route will still match and the - * `$location.path` will be updated to add or drop the trailing slash to exacly match the + * `$location.path` will be updated to add or drop the trailing slash to exactly match the * route definition. + * + * `path` can contain named groups starting with a colon (`:name`). All characters up to the + * next slash are matched and stored in `$routeParams` under the given `name` when the route + * matches. + * * @param {Object} route Mapping information to be assigned to `$route.current` on route * match. * @@ -286,8 +291,7 @@ function $RouteProvider(){ * instance of the Controller. */ - var matcher = switchRouteMatcher, - forceReload = false, + var forceReload = false, $route = { routes: routes, @@ -315,21 +319,36 @@ function $RouteProvider(){ ///////////////////////////////////////////////////// + /** + * @param on {string} current url + * @param when {string} route when template to match the url against + * @return {?Object} + */ function switchRouteMatcher(on, when) { // TODO(i): this code is convoluted and inefficient, we should construct the route matching // regex only once and then reuse it - var regex = '^' + when.replace(/([\.\\\(\)\^\$])/g, "\\$1") + '$', + + // Escape regexp special characters. + when = '^' + when.replace(/[-\/\\^$*+?.()|[\]{}]/g, "\\$&") + '$'; + var regex = '', params = [], dst = {}; - forEach(when.split(/[^\w:]/), function(param) { - if (param && param.charAt(0) === ':') { - var paramRegExp = new RegExp(param + "([\\W])"); - if (regex.match(paramRegExp)) { - regex = regex.replace(paramRegExp, "([^\\/]*)$1"); - params.push(param.substr(1)); - } - } - }); + + var re = /:(\w+)/g, + paramMatch, + lastMatchedIndex = 0; + + while ((paramMatch = re.exec(when)) !== null) { + // Find each :param in `when` and replace it with a capturing group. + // Append all other sections of when unchanged. + regex += when.slice(lastMatchedIndex, paramMatch.index); + regex += '([^\\/]*)'; + params.push(paramMatch[1]); + lastMatchedIndex = re.lastIndex; + } + // Append trailing path part. + regex += when.substr(lastMatchedIndex); + var match = on.match(new RegExp(regex)); if (match) { forEach(params, function(name, index) { @@ -418,7 +437,7 @@ function $RouteProvider(){ // Match a route var params, match; forEach(routes, function(route, path) { - if (!match && (params = matcher($location.path(), path))) { + if (!match && (params = switchRouteMatcher($location.path(), path))) { match = inherit(route, { params: extend({}, $location.search(), params), pathParams: params}); diff --git a/test/ng/routeParamsSpec.js b/test/ng/routeParamsSpec.js index 07c6d4f79471..52fe8d2a4a08 100644 --- a/test/ng/routeParamsSpec.js +++ b/test/ng/routeParamsSpec.js @@ -29,4 +29,16 @@ describe('$routeParams', function() { expect($routeParams).toEqual({bar:'barvalue', foo:'foovalue'}); }); }); + + it('should support route params not preceded by slashes', function() { + module(function($routeProvider) { + $routeProvider.when('/bar:barId/foo:fooId/', {}); + }); + + inject(function($rootScope, $route, $location, $routeParams) { + $location.path('/barbarvalue/foofoovalue/'); + $rootScope.$digest(); + expect($routeParams).toEqual({barId: 'barvalue', fooId: 'foovalue'}); + }); + }); }); diff --git a/test/ng/routeSpec.js b/test/ng/routeSpec.js index 52fffa406b69..852f97b5231c 100644 --- a/test/ng/routeSpec.js +++ b/test/ng/routeSpec.js @@ -82,28 +82,40 @@ describe('$route', function() { }); - it('should match a route that contains special chars in the path', function() { - module(function($routeProvider) { - $routeProvider.when('/$test.23/foo(bar)/:baz', {templateUrl: 'test.html'}); - }); - inject(function($route, $location, $rootScope) { + describe('should match a route that contains special chars in the path', function() { + beforeEach(module(function($routeProvider) { + $routeProvider.when('/$test.23/foo*(bar)/:baz', {templateUrl: 'test.html'}); + })); + it('matches the full path', inject(function($route, $location, $rootScope) { $location.path('/test'); $rootScope.$digest(); expect($route.current).toBeUndefined(); + })); - $location.path('/$testX23/foo(bar)/222'); + it('matches literal .', inject(function($route, $location, $rootScope) { + $location.path('/$testX23/foo*(bar)/222'); $rootScope.$digest(); expect($route.current).toBeUndefined(); + })); - $location.path('/$test.23/foo(bar)/222'); + it('matches literal *', inject(function($route, $location, $rootScope) { + $location.path('/$test.23/foooo(bar)/222'); $rootScope.$digest(); - expect($route.current).toBeDefined(); + expect($route.current).toBeUndefined(); + })); - $location.path('/$test.23/foo\\(bar)/222'); + it('treats backslashes normally', inject(function($route, $location, $rootScope) { + $location.path('/$test.23/foo*\\(bar)/222'); $rootScope.$digest(); expect($route.current).toBeUndefined(); - }); + })); + + it('matches a URL with special chars', inject(function($route, $location, $rootScope) { + $location.path('/$test.23/foo*(bar)/222'); + $rootScope.$digest(); + expect($route.current).toBeDefined(); + })); });