Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit ca5b27b

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 filtering out comment nodes in the `addOption()` method. Fixes #13874 Closes #13878
1 parent 3d158f6 commit ca5b27b

File tree

2 files changed

+30
-4
lines changed

2 files changed

+30
-4
lines changed

src/ng/directive/select.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ var SelectController =
8080

8181
// Tell the select control that an option, with the given value, has been added
8282
self.addOption = function(value, element) {
83+
// Skip comment nodes, as they only pollute the `optionsMap`
84+
if (element[0].nodeType === NODE_TYPE_COMMENT) return;
85+
8386
assertNotHasOwnProperty(value, '"option value"');
8487
if (value === '') {
8588
self.emptyOption = element;
@@ -452,7 +455,6 @@ var optionDirective = ['$interpolate', function($interpolate) {
452455
restrict: 'E',
453456
priority: 100,
454457
compile: function(element, attr) {
455-
456458
if (isDefined(attr.value)) {
457459
// If the value attribute is defined, check if it contains an interpolation
458460
var interpolateValueFn = $interpolate(attr.value, true);
@@ -466,7 +468,6 @@ var optionDirective = ['$interpolate', function($interpolate) {
466468
}
467469

468470
return function(scope, element, attr) {
469-
470471
// This is an optimization over using ^^ since we don't want to have to search
471472
// all the way to the root of the DOM for every single option element
472473
var selectCtrlName = '$selectController',

test/ng/directive/selectSpec.js

+27-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,20 @@ 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+
compile('<select ng-model="mySelect"><option my-options="options"></option></select>');
635+
636+
expect(element).toEqualSelect([unknownValue()], '1', '2', '3');
637+
}
638+
);
614639
});
615640

616641

0 commit comments

Comments
 (0)