From 64d994b68a927d7ce30d2a35d36d8322adef5d5a Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 6 Oct 2015 22:07:25 +0200 Subject: [PATCH] fix(ngOptions): provide robust identification of empty options with ngIf When an empty option has ngIf, the compilation result is different from the extracted emptyOption, so the default way of identifying the empty option doesn't work anymore. In that case, we need to make sure that we don't identify comment nodes and options whose value is '' as valid option elements. Related #12952 Closes #12190 --- src/ng/directive/ngOptions.js | 18 ++++++++++-- test/ng/directive/ngOptionsSpec.js | 47 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 26f2f0cf1904..6ede818f68f0 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -401,6 +401,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { // The emptyOption allows the application developer to provide their own custom "empty" // option when the viewValue does not match any of the option values. var emptyOption; + // The empty option might have a directive that dynamically adds / removes it + var dynamicEmptyOption; + for (var i = 0, children = selectElement.children(), ii = children.length; i < ii; i++) { if (children[i].value === '') { emptyOption = children.eq(i); @@ -550,6 +553,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { // compile the element since there might be bindings in it $compile(emptyOption)(scope); + if (emptyOption[0].nodeType === NODE_TYPE_COMMENT) { + dynamicEmptyOption = true; + } // remove the class, which is added automatically because we recompile the element and it // becomes the compilation root @@ -619,9 +625,15 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { var unknownOption_ = unknownOption && unknownOption[0]; if (emptyOption_ || unknownOption_) { - while (current && - (current === emptyOption_ || - current === unknownOption_)) { + + while ( + current && + (current === emptyOption_ || + current === unknownOption_ || + // When the empty option is dynamically added / removed, we cannot be sure that the + // emptyOption is the same as it was when it was extracted in the first place + dynamicEmptyOption && (current.nodeType === NODE_TYPE_COMMENT || current.value === '')) + ) { current = current.nextSibling; } } diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index c5dac4b72914..c0ea84a408e1 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -2144,6 +2144,53 @@ describe('ngOptions', function() { }); + it('should be possible to use ngIf in the blank option', function() { + var option; + createSingleSelect(''); + + scope.$apply(function() { + scope.values = [{name: 'A'}]; + scope.isBlank = true; + }); + + expect(element.find('option').length).toBe(2); + option = element.find('option').eq(0); + expect(option.val()).toBe(''); + expect(option.text()).toBe('blank'); + + scope.$apply(function() { + scope.isBlank = false; + }); + + expect(element.find('option').length).toBe(1); + option = element.find('option').eq(0); + expect(option.text()).toBe('A'); + }); + + + it('should be possible to use ngIf in the blank option when values are available upon linking', + function() { + var options; + + scope.values = [{name: 'A'}]; + createSingleSelect(''); + + scope.$apply('isBlank = true'); + + options = element.find('option'); + expect(options.length).toBe(2); + expect(options.eq(0).val()).toBe(''); + expect(options.eq(0).text()).toBe('blank'); + + scope.$apply('isBlank = false'); + + options = element.find('option'); + expect(options.length).toBe(1); + expect(options.eq(0).text()).toBe('A'); + } + ); + + it('should not throw when a directive compiles the blank option before ngOptions is linked', function() { expect(function() { createSelect({