From 4622af3f075204e2d5ab33d5bd002074f2d940c9 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 19 Mar 2013 10:22:06 +0000 Subject: [PATCH] fix(select): ensure empty option is not lost in IE9 Fix a check inside render for select elements with ngOptions, which compares the selected property of an element with it's desired state. Ensure the placeholder, if available, is explicitly selected if the model value can not be found in the option list. Without these fixes it's up to the browser implementation to decide which option to choose. In most browsers, this has the effect of displaying the first item in the list. In IE9 however, this causes the select to display nothing. Closes #2150, #1826 --- src/ng/directive/select.js | 18 ++++++++++-------- test/ng/directive/selectSpec.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 73671e03cc7e..4f4aace29557 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -395,10 +395,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { if (multiple) { selectedSet = new HashMap(modelValue); - } else if (modelValue === null || nullOption) { - // if we are not multiselect, and we are null then we have to add the nullOption - optionGroups[''].push({selected:modelValue === null, id:'', label:''}); - selectedSet = true; } // We now build up the list of options we need (we merge later) @@ -423,9 +419,14 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { selected: selected // determine if we should be selected }); } - if (!multiple && !selectedSet) { - // nothing was selected, we have to insert the undefined item - optionGroups[''].unshift({id:'?', label:'', selected:true}); + if (!multiple) { + if (nullOption || modelValue === null) { + // insert null option if we have a placeholder, or the model is null + optionGroups[''].unshift({id:'', label:'', selected:!selectedSet}); + } else if (!selectedSet) { + // option could not be found, we have to insert the undefined item + optionGroups[''].unshift({id:'?', label:'', selected:true}); + } } // Now we need to update the list of DOM nodes to match the optionGroups we computed above @@ -469,7 +470,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { if (existingOption.id !== option.id) { lastElement.val(existingOption.id = option.id); } - if (existingOption.element.selected !== option.selected) { + // lastElement.prop('selected') provided by jQuery has side-effects + if (lastElement[0].selected !== option.selected) { lastElement.prop('selected', (existingOption.selected = option.selected)); } } else { diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 2b56228d17c9..91d09893e685 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -977,6 +977,19 @@ describe('select', function() { expect(option.attr('id')).toBe('road-runner'); expect(option.attr('custom-attr')).toBe('custom-attr'); }); + + it('should be selected, if it is available and no other option is selected', function() { + // selectedIndex is used here because jqLite incorrectly reports element.val() + scope.$apply(function() { + scope.values = [{name: 'A'}]; + }); + createSingleSelect(true); + // ensure the first option (the blank option) is selected + expect(element[0].selectedIndex).toEqual(0); + scope.$digest(); + // ensure the option has not changed following the digest + expect(element[0].selectedIndex).toEqual(0); + }); }); @@ -1099,6 +1112,21 @@ describe('select', function() { browserTrigger(element, 'change'); expect(scope.selected).toEqual(['0']); }); + + it('should deselect all options when model is emptied', function() { + createMultiSelect(); + scope.$apply(function() { + scope.values = [{name: 'A'}, {name: 'B'}]; + scope.selected = [scope.values[0]]; + }); + expect(element.find('option')[0].selected).toEqual(true); + + scope.$apply(function() { + scope.selected.pop(); + }); + + expect(element.find('option')[0].selected).toEqual(false); + }) });