Skip to content

Commit 64d994b

Browse files
committed
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 angular#12952 Closes angular#12190
1 parent beea571 commit 64d994b

File tree

2 files changed

+62
-3
lines changed

2 files changed

+62
-3
lines changed

src/ng/directive/ngOptions.js

+15-3
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
401401
// The emptyOption allows the application developer to provide their own custom "empty"
402402
// option when the viewValue does not match any of the option values.
403403
var emptyOption;
404+
// The empty option might have a directive that dynamically adds / removes it
405+
var dynamicEmptyOption;
406+
404407
for (var i = 0, children = selectElement.children(), ii = children.length; i < ii; i++) {
405408
if (children[i].value === '') {
406409
emptyOption = children.eq(i);
@@ -550,6 +553,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
550553

551554
// compile the element since there might be bindings in it
552555
$compile(emptyOption)(scope);
556+
if (emptyOption[0].nodeType === NODE_TYPE_COMMENT) {
557+
dynamicEmptyOption = true;
558+
}
553559

554560
// remove the class, which is added automatically because we recompile the element and it
555561
// becomes the compilation root
@@ -619,9 +625,15 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
619625
var unknownOption_ = unknownOption && unknownOption[0];
620626

621627
if (emptyOption_ || unknownOption_) {
622-
while (current &&
623-
(current === emptyOption_ ||
624-
current === unknownOption_)) {
628+
629+
while (
630+
current &&
631+
(current === emptyOption_ ||
632+
current === unknownOption_ ||
633+
// When the empty option is dynamically added / removed, we cannot be sure that the
634+
// emptyOption is the same as it was when it was extracted in the first place
635+
dynamicEmptyOption && (current.nodeType === NODE_TYPE_COMMENT || current.value === ''))
636+
) {
625637
current = current.nextSibling;
626638
}
627639
}

test/ng/directive/ngOptionsSpec.js

+47
Original file line numberDiff line numberDiff line change
@@ -2144,6 +2144,53 @@ describe('ngOptions', function() {
21442144
});
21452145

21462146

2147+
it('should be possible to use ngIf in the blank option', function() {
2148+
var option;
2149+
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
2150+
2151+
scope.$apply(function() {
2152+
scope.values = [{name: 'A'}];
2153+
scope.isBlank = true;
2154+
});
2155+
2156+
expect(element.find('option').length).toBe(2);
2157+
option = element.find('option').eq(0);
2158+
expect(option.val()).toBe('');
2159+
expect(option.text()).toBe('blank');
2160+
2161+
scope.$apply(function() {
2162+
scope.isBlank = false;
2163+
});
2164+
2165+
expect(element.find('option').length).toBe(1);
2166+
option = element.find('option').eq(0);
2167+
expect(option.text()).toBe('A');
2168+
});
2169+
2170+
2171+
it('should be possible to use ngIf in the blank option when values are available upon linking',
2172+
function() {
2173+
var options;
2174+
2175+
scope.values = [{name: 'A'}];
2176+
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
2177+
2178+
scope.$apply('isBlank = true');
2179+
2180+
options = element.find('option');
2181+
expect(options.length).toBe(2);
2182+
expect(options.eq(0).val()).toBe('');
2183+
expect(options.eq(0).text()).toBe('blank');
2184+
2185+
scope.$apply('isBlank = false');
2186+
2187+
options = element.find('option');
2188+
expect(options.length).toBe(1);
2189+
expect(options.eq(0).text()).toBe('A');
2190+
}
2191+
);
2192+
2193+
21472194
it('should not throw when a directive compiles the blank option before ngOptions is linked', function() {
21482195
expect(function() {
21492196
createSelect({

0 commit comments

Comments
 (0)