Skip to content

Commit 39f21f3

Browse files
committed
fix(select): handle corner case of adding options via a custom directive
Under specific circumstances (e.g. adding options via a directive with `replace: true` and a structural directive in its template), an error occurred when trying to call `hasAttribute()` on a comment node (which doesn't support that method). This commit fixes it by first checking if `hasAttribute()` is available. Fixes angular#13874
1 parent 5a3504a commit 39f21f3

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-4
lines changed

src/ng/directive/select.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ function chromeHack(optionElement) {
66
// Workaround for https://code.google.com/p/chromium/issues/detail?id=381459
77
// Adding an <option selected="selected"> element to a <select required="required"> should
88
// automatically select the new element
9-
if (optionElement[0].hasAttribute('selected')) {
10-
optionElement[0].selected = true;
9+
var optionNode = optionElement[0];
10+
if (optionNode.hasAttribute && optionNode.hasAttribute('selected')) {
11+
optionNode.selected = true;
1112
}
1213
}
1314

test/ng/directive/selectSpec.js

+31-2
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ describe('select', function() {
4444
}
4545
};
4646
});
47+
48+
$compileProvider.directive('myOptions', function() {
49+
return {
50+
scope: {myOptions: '='},
51+
replace: true,
52+
template:
53+
'<option value="{{ ::option.value }}" ng-repeat="option in ::myOptions">' +
54+
'{{ ::options.label }}' +
55+
'</option>'
56+
};
57+
});
4758
}));
4859

4960
beforeEach(inject(function($rootScope, _$compile_) {
@@ -313,7 +324,7 @@ describe('select', function() {
313324
});
314325

315326

316-
it('should cope with a dynamic empty option added to a static empty option', function() {
327+
it('should cope with a dynamic empty option added to a static empty option', function() {
317328
scope.dynamicOptions = [];
318329
scope.robot = 'x';
319330
compile('<select ng-model="robot">' +
@@ -340,7 +351,7 @@ describe('select', function() {
340351
scope.dynamicOptions = [];
341352
scope.$digest();
342353
expect(element).toEqualSelect(['']);
343-
});
354+
});
344355

345356
it('should select the empty option when model is undefined', function() {
346357
compile('<select ng-model="robot">' +
@@ -611,6 +622,24 @@ describe('select', function() {
611622

612623
});
613624

625+
626+
it('should not break when adding options via a directive with `replace: true` '
627+
+ 'and a structural directive in its template',
628+
function() {
629+
scope.options = [
630+
{value: '1', label: 'Option 1'},
631+
{value: '2', label: 'Option 2'},
632+
{value: '3', label: 'Option 3'},
633+
];
634+
635+
expect(function () {
636+
compile(
637+
'<select ng-model="mySelect">' +
638+
'<option my-options="options"></option>' +
639+
'</select>');
640+
}).not.toThrow();
641+
}
642+
);
614643
});
615644

616645

0 commit comments

Comments
 (0)