diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index fdef4d779245..2a5ed5e205f0 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -80,6 +80,9 @@ var SelectController = // Tell the select control that an option, with the given value, has been added self.addOption = function(value, element) { + // Skip comment nodes, as they only pollute the `optionsMap` + if (element[0].nodeType === NODE_TYPE_COMMENT) return; + assertNotHasOwnProperty(value, '"option value"'); if (value === '') { self.emptyOption = element; @@ -452,7 +455,6 @@ var optionDirective = ['$interpolate', function($interpolate) { restrict: 'E', priority: 100, compile: function(element, attr) { - if (isDefined(attr.value)) { // If the value attribute is defined, check if it contains an interpolation var interpolateValueFn = $interpolate(attr.value, true); @@ -466,7 +468,6 @@ var optionDirective = ['$interpolate', function($interpolate) { } return function(scope, element, attr) { - // This is an optimization over using ^^ since we don't want to have to search // all the way to the root of the DOM for every single option element var selectCtrlName = '$selectController', diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 23a961611cce..7749d2a976db 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -44,6 +44,17 @@ describe('select', function() { } }; }); + + $compileProvider.directive('myOptions', function() { + return { + scope: {myOptions: '='}, + replace: true, + template: + '' + }; + }); })); beforeEach(inject(function($rootScope, _$compile_) { @@ -313,7 +324,7 @@ describe('select', function() { }); - it('should cope with a dynamic empty option added to a static empty option', function() { + it('should cope with a dynamic empty option added to a static empty option', function() { scope.dynamicOptions = []; scope.robot = 'x'; compile('' + @@ -611,6 +622,20 @@ describe('select', function() { }); + + it('should not break when adding options via a directive with `replace: true` ' + + 'and a structural directive in its template', + function() { + scope.options = [ + {value: '1', label: 'Option 1'}, + {value: '2', label: 'Option 2'}, + {value: '3', label: 'Option 3'} + ]; + compile(''); + + expect(element).toEqualSelect([unknownValue()], '1', '2', '3'); + } + ); });