X Tutup
Skip to content

Commit 69c1405

Browse files
davidreherpetebacondarwin
davidreher
authored and
petebacondarwin
committed
fix(angular_1_router): ng-link is generating wrong hrefs
At the moment ng-link is generating html5mode URLs for `href`s. Instead it should check whether or not html5mode is enabled and create the `href`s accordingly. The renaming in the `getLink` function is aligning it to `RouterLink`'s `_updateLink`. Closes #7423
1 parent 980491b commit 69c1405

File tree

3 files changed

+104
-103
lines changed

3 files changed

+104
-103
lines changed

‎modules/angular1_router/lib/facades.es5‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,13 @@ Location.prototype.path = function () {
316316
Location.prototype.go = function (path, query) {
317317
return $location.url(path + query);
318318
};
319+
Location.prototype.prepareExternalUrl = function(url) {
320+
if (url.length > 0 && !url.startsWith('/')) {
321+
url = '/' + url;
322+
}
323+
if(!$location.$$html5) {
324+
return '#' + url;
325+
} else {
326+
return '.' + url;
327+
}
328+
};

‎modules/angular1_router/src/ng_outlet.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,13 +207,13 @@ function ngLinkDirective($rootRouter, $parse) {
207207
return;
208208
}
209209

210-
let instruction = null;
210+
let navigationInstruction = null;
211211
let link = attrs.ngLink || '';
212212

213213
function getLink(params) {
214-
instruction = router.generate(params);
214+
navigationInstruction = router.generate(params);
215215

216-
scope.$watch(function() { return router.isRouteActive(instruction); },
216+
scope.$watch(function() { return router.isRouteActive(navigationInstruction); },
217217
function(active) {
218218
if (active) {
219219
element.addClass('ng-link-active');
@@ -222,7 +222,8 @@ function ngLinkDirective($rootRouter, $parse) {
222222
}
223223
});
224224

225-
return './' + angular.stringifyInstruction(instruction);
225+
const navigationHref = navigationInstruction.toLinkUrl();
226+
return $rootRouter._location.prepareExternalUrl(navigationHref);
226227
}
227228

228229
let routeParamsGetter = $parse(link);
@@ -236,11 +237,11 @@ function ngLinkDirective($rootRouter, $parse) {
236237
}
237238

238239
element.on('click', event => {
239-
if (event.which !== 1 || !instruction) {
240+
if (event.which !== 1 || !navigationInstruction) {
240241
return;
241242
}
242243

243-
$rootRouter.navigateByInstruction(instruction);
244+
$rootRouter.navigateByInstruction(navigationInstruction);
244245
event.preventDefault();
245246
});
246247
}

‎modules/angular1_router/test/ng_link_spec.js‎

Lines changed: 87 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -2,183 +2,173 @@
22

33
describe('ngLink', function () {
44

5-
var elt,
6-
$compile,
7-
$rootScope,
8-
$rootRouter,
9-
$compileProvider;
10-
11-
beforeEach(function () {
12-
module('ng');
13-
module('ngComponentRouter');
14-
module(function (_$compileProvider_) {
15-
$compileProvider = _$compileProvider_;
16-
});
17-
18-
inject(function (_$compile_, _$rootScope_, _$rootRouter_) {
19-
$compile = _$compile_;
20-
$rootScope = _$rootScope_;
21-
$rootRouter = _$rootRouter_;
22-
});
23-
24-
registerComponent('userCmp', '<div>hello {{userCmp.$routeParams.name}}</div>', function () {});
25-
registerComponent('oneCmp', '<div>{{oneCmp.number}}</div>', function () {this.number = 'one'});
26-
registerComponent('twoCmp', '<div><a ng-link="[\'/Two\']">{{twoCmp.number}}</a></div>', function () {this.number = 'two'});
27-
});
28-
29-
305
it('should allow linking from the parent to the child', function () {
31-
$rootRouter.config([
6+
setup();
7+
configureRouter([
328
{ path: '/a', component: 'oneCmp' },
339
{ path: '/b', component: 'twoCmp', name: 'Two' }
3410
]);
35-
compile('<a ng-link="[\'/Two\']">link</a> | outer { <div ng-outlet></div> }');
36-
37-
$rootRouter.navigateByUrl('/a');
38-
$rootScope.$digest();
3911

12+
var elt = compile('<a ng-link="[\'/Two\']">link</a> | outer { <div ng-outlet></div> }');
13+
navigateTo('/a');
4014
expect(elt.find('a').attr('href')).toBe('./b');
4115
});
4216

4317
it('should allow linking from the child and the parent', function () {
44-
$rootRouter.config([
18+
setup();
19+
configureRouter([
4520
{ path: '/a', component: 'oneCmp' },
4621
{ path: '/b', component: 'twoCmp', name: 'Two' }
4722
]);
48-
compile('outer { <div ng-outlet></div> }');
49-
50-
$rootRouter.navigateByUrl('/b');
51-
$rootScope.$digest();
5223

24+
var elt = compile('outer { <div ng-outlet></div> }');
25+
navigateTo('/b');
5326
expect(elt.find('a').attr('href')).toBe('./b');
5427
});
5528

5629

5730
it('should allow params in routerLink directive', function () {
31+
setup();
5832
registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: \'lol\'}]">{{twoLinkCmp.number}}</a></div>', function () {this.number = 'two'});
59-
60-
$rootRouter.config([
33+
configureRouter([
6134
{ path: '/a', component: 'twoLinkCmp' },
6235
{ path: '/b/:param', component: 'twoCmp', name: 'Two' }
6336
]);
64-
compile('<div ng-outlet></div>');
65-
66-
$rootRouter.navigateByUrl('/a');
67-
$rootScope.$digest();
6837

38+
var elt = compile('<div ng-outlet></div>');
39+
navigateTo('/a');
6940
expect(elt.find('a').attr('href')).toBe('./b/lol');
7041
});
7142

7243

7344
it('should update the href of links with bound params', function () {
74-
registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: twoLinkCmp.number}]">{{twoLinkCmp.number}}</a></div>', function () {this.number = 'param'});
75-
$rootRouter.config([
45+
setup();
46+
registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: $ctrl.number}]">{{$ctrl.number}}</a></div>', function () {this.number = 43});
47+
configureRouter([
7648
{ path: '/a', component: 'twoLinkCmp' },
7749
{ path: '/b/:param', component: 'twoCmp', name: 'Two' }
7850
]);
79-
compile('<div ng-outlet></div>');
80-
81-
$rootRouter.navigateByUrl('/a');
82-
$rootScope.$digest();
8351

84-
expect(elt.find('a').attr('href')).toBe('./b/param');
52+
var elt = compile('<div ng-outlet></div>');
53+
navigateTo('/a');
54+
expect(elt.find('a').text()).toBe('43');
55+
expect(elt.find('a').attr('href')).toBe('./b/43');
8556
});
8657

8758

8859
it('should navigate on left-mouse click when a link url matches a route', function () {
89-
$rootRouter.config([
60+
setup();
61+
configureRouter([
9062
{ path: '/', component: 'oneCmp' },
9163
{ path: '/two', component: 'twoCmp', name: 'Two'}
9264
]);
9365

94-
compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
95-
$rootScope.$digest();
66+
var elt = compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
9667
expect(elt.text()).toBe('link | one');
97-
9868
expect(elt.find('a').attr('href')).toBe('./two');
9969

10070
elt.find('a')[0].click();
101-
102-
$rootScope.$digest();
71+
inject(function($rootScope) { $rootScope.$digest(); });
10372
expect(elt.text()).toBe('link | two');
10473
});
10574

10675

107-
it('should not navigate on non-left mouse click when a link url matches a route', inject(function ($rootRouter) {
108-
$rootRouter.config([
76+
it('should not navigate on non-left mouse click when a link url matches a route', function() {
77+
setup();
78+
configureRouter([
10979
{ path: '/', component: 'oneCmp' },
11080
{ path: '/two', component: 'twoCmp', name: 'Two'}
11181
]);
11282

113-
compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
114-
$rootScope.$digest();
83+
var elt = compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
11584
expect(elt.text()).toBe('link | one');
11685
elt.find('a').triggerHandler({ type: 'click', which: 3 });
117-
118-
$rootScope.$digest();
86+
inject(function($rootScope) { $rootScope.$digest(); });
11987
expect(elt.text()).toBe('link | one');
120-
}));
88+
});
12189

12290

12391
// See https://github.com/angular/router/issues/206
12492
it('should not navigate a link without an href', function () {
125-
$rootRouter.config([
93+
setup();
94+
configureRouter([
12695
{ path: '/', component: 'oneCmp' },
12796
{ path: '/two', component: 'twoCmp', name: 'Two'}
12897
]);
12998
expect(function () {
130-
compile('<a>link</a>');
131-
$rootScope.$digest();
99+
var elt = compile('<a>link</a>');
132100
expect(elt.text()).toBe('link');
133101
elt.find('a')[0].click();
134-
$rootScope.$digest();
102+
inject(function($rootScope) { $rootScope.$digest(); });
135103
}).not.toThrow();
136104
});
137105

138-
it('should add an ng-link-active class on the current link', inject(function ($rootRouter) {
139-
$rootRouter.config([
106+
it('should add an ng-link-active class on the current link', function() {
107+
setup();
108+
configureRouter([
140109
{ path: '/', component: 'oneCmp', name: 'One' }
141110
]);
142111

143-
compile('<a ng-link="[\'/One\']">one</a> | <div ng-outlet></div>');
144-
$rootScope.$digest();
145-
146-
$rootRouter.navigateByUrl('/');
147-
$rootScope.$digest();
148-
112+
var elt = compile('<a ng-link="[\'/One\']">one</a> | <div ng-outlet></div>');
113+
navigateTo('/');
149114
expect(elt.find('a').attr('class')).toBe('ng-link-active');
150-
}));
115+
});
116+
151117

118+
describe('html5Mode disabled', function () {
119+
it('should prepend href with a hash', function () {
120+
setup({ html5Mode: false });
121+
module(function($locationProvider) {
122+
$locationProvider.html5Mode(false);
123+
});
124+
configureRouter([
125+
{ path: '/b', component: 'twoCmp', name: 'Two' }
126+
]);
127+
var elt = compile('<a ng-link="[\'/Two\']">link</a>');
128+
expect(elt.find('a').attr('href')).toBe('#/b');
129+
});
130+
});
152131

153-
function registerComponent(name, template, config) {
154-
var controller = function () {};
155132

156-
function factory() {
157-
return {
133+
function registerComponent(name, template, controller) {
134+
module(function($compileProvider) {
135+
$compileProvider.component(name, {
158136
template: template,
159-
controllerAs: name,
160137
controller: controller
161-
};
162-
}
163-
164-
if (!template) {
165-
template = '';
166-
}
167-
if (angular.isArray(config)) {
168-
factory.annotations = [new angular.annotations.RouteConfig(config)];
169-
} else if (typeof config === 'function') {
170-
controller = config;
171-
} else if (typeof config === 'object') {
172-
if (config.canActivate) {
173-
controller.$canActivate = config.canActivate;
174-
}
175-
}
176-
$compileProvider.directive(name, factory);
138+
});
139+
});
140+
}
141+
142+
function setup(config) {
143+
var html5Mode = !(config && config.html5Mode === false);
144+
module('ngComponentRouter')
145+
module(function($locationProvider) {
146+
$locationProvider.html5Mode(html5Mode);
147+
});
148+
registerComponent('userCmp', '<div>hello {{$ctrl.$routeParams.name}}</div>', function () {});
149+
registerComponent('oneCmp', '<div>{{$ctrl.number}}</div>', function () {this.number = 'one'});
150+
registerComponent('twoCmp', '<div><a ng-link="[\'/Two\']">{{$ctrl.number}}</a></div>', function () {this.number = 'two'});
151+
}
152+
153+
function configureRouter(routeConfig) {
154+
inject(function($rootRouter) {
155+
$rootRouter.config(routeConfig);
156+
});
177157
}
178158

179159
function compile(template) {
180-
elt = $compile('<div>' + template + '</div>')($rootScope);
181-
$rootScope.$digest();
160+
var elt;
161+
inject(function($compile, $rootScope) {
162+
elt = $compile('<div>' + template + '</div>')($rootScope);
163+
$rootScope.$digest();
164+
});
182165
return elt;
183166
}
167+
168+
function navigateTo(url) {
169+
inject(function($rootRouter, $rootScope) {
170+
$rootRouter.navigateByUrl(url);
171+
$rootScope.$digest();
172+
});
173+
}
184174
});

0 commit comments

Comments
 (0)
X Tutup