diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 0c8201068c29..4755a85932f5 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -4,6 +4,18 @@ var noopNgModelController = { $setViewValue: noop, $render: noop }; +function setOptionSelectedStatus(optionEl, value) { + optionEl.prop('selected', value); // needed for IE + /** + * When unselecting an option, setting the property to null / false should be enough + * However, screenreaders might react to the selected attribute instead, see + * https://github.com/angular/angular.js/issues/14419 + * Note: "selected" is a boolean attr and will be removed when the "value" arg in attr() is false + * or null + */ + optionEl.attr('selected', value); +} + /** * @ngdoc type * @name select.SelectController @@ -44,14 +56,14 @@ var SelectController = var unknownVal = self.generateUnknownOptionValue(val); self.unknownOption.val(unknownVal); $element.prepend(self.unknownOption); - setOptionAsSelected(self.unknownOption); + setOptionSelectedStatus(self.unknownOption, true); $element.val(unknownVal); }; self.updateUnknownOption = function(val) { var unknownVal = self.generateUnknownOptionValue(val); self.unknownOption.val(unknownVal); - setOptionAsSelected(self.unknownOption); + setOptionSelectedStatus(self.unknownOption, true); $element.val(unknownVal); }; @@ -66,7 +78,7 @@ var SelectController = self.selectEmptyOption = function() { if (self.emptyOption) { $element.val(''); - setOptionAsSelected(self.emptyOption); + setOptionSelectedStatus(self.emptyOption, true); } }; @@ -102,7 +114,7 @@ var SelectController = // Make sure to remove the selected attribute from the previously selected option // Otherwise, screen readers might get confused var currentlySelectedOption = $element[0].options[$element[0].selectedIndex]; - if (currentlySelectedOption) currentlySelectedOption.removeAttribute('selected'); + if (currentlySelectedOption) setOptionSelectedStatus(jqLite(currentlySelectedOption), false); if (self.hasOption(value)) { self.removeUnknownOption(); @@ -112,7 +124,7 @@ var SelectController = // Set selected attribute and property on selected option for screen readers var selectedOption = $element[0].options[$element[0].selectedIndex]; - setOptionAsSelected(jqLite(selectedOption)); + setOptionSelectedStatus(jqLite(selectedOption), true); } else { if (value == null && self.emptyOption) { self.removeUnknownOption(); @@ -292,11 +304,6 @@ var SelectController = } }); }; - - function setOptionAsSelected(optionEl) { - optionEl.prop('selected', true); // needed for IE - optionEl.attr('selected', true); - } }]; /** @@ -607,8 +614,20 @@ var selectDirective = function() { // Write value now needs to set the selected property of each matching option selectCtrl.writeValue = function writeMultipleValue(value) { forEach(element.find('option'), function(option) { - option.selected = !!value && (includes(value, option.value) || - includes(value, selectCtrl.selectValueMap[option.value])); + var shouldBeSelected = !!value && (includes(value, option.value) || + includes(value, selectCtrl.selectValueMap[option.value])); + var currentlySelected = option.selected; + + // IE and Edge, adding options to the selection via shift+click/UP/DOWN, + // will de-select already selected options if "selected" on those options was set + // more than once (i.e. when the options were already selected) + // So we only modify the selected property if neccessary. + // Note: this behavior cannot be replicated via unit tests because it only shows in the + // actual user interface. + if (shouldBeSelected !== currentlySelected) { + setOptionSelectedStatus(jqLite(option), shouldBeSelected); + } + }); }; diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index e77d5bffcf3f..975e4b594d77 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -1098,13 +1098,21 @@ describe('select', function() { scope.selection = ['A']; }); + var optionElements = element.find('option'); + expect(element).toEqualSelect(['A'], 'B'); + expect(optionElements[0]).toBeMarkedAsSelected(); + expect(optionElements[1]).not.toBeMarkedAsSelected(); scope.$apply(function() { scope.selection.push('B'); }); + optionElements = element.find('option'); + expect(element).toEqualSelect(['A'], ['B']); + expect(optionElements[0]).toBeMarkedAsSelected(); + expect(optionElements[1]).toBeMarkedAsSelected(); }); it('should work with optgroups', function() {