diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 10d970097f02..781d68b423d1 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -448,10 +448,11 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile, var removeEmptyOption = function() { if (!providedEmptyOption) { emptyOption.remove(); + } else { + emptyOption.removeAttr('selected'); } }; - var renderUnknownOption = function() { selectElement.prepend(unknownOption); selectElement.val('?'); @@ -467,8 +468,13 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile, if (!multiple) { selectCtrl.writeValue = function writeNgOptionsValue(value) { + var selectedOption = options.selectValueMap[selectElement.val()]; var option = options.getOptionFromViewValue(value); + // Make sure to remove the selected attribute from the previously selected option + // Otherwise, screen readers might get confused + if (selectedOption) selectedOption.element.removeAttribute('selected'); + if (option) { // Don't update the option when it is already selected. // For example, the browser will select the first option by default. In that case, @@ -509,6 +515,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile, // If we are using `track by` then we must watch the tracked value on the model // since ngModel only watches for object identity change + // FIXME: When a user selects an option, this watch will fire needlessly if (ngOptions.trackBy) { scope.$watch( function() { return ngOptions.getTrackByValue(ngModelCtrl.$viewValue); }, diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 8fb6a2a9fa3f..6eab57ff8fa3 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -120,6 +120,55 @@ describe('ngOptions', function() { return { pass: errors.length === 0, message: message }; } }; + }, + toBeMarkedAsSelected: function() { + // Selected is special because the element property and attribute reflect each other's state. + // IE9 will wrongly report hasAttribute('selected') === true when the property is + // undefined or null, and the dev tools show that no attribute is set + return { + compare: function(actual) { + var errors = []; + if (actual.selected === null || typeof actual.selected === 'undefined' || actual.selected === false) { + errors.push('Expected option property "selected" to be truthy'); + } + + if (msie !== 9 && actual.hasAttribute('selected') === false) { + errors.push('Expected option to have attribute "selected"'); + } + + var message = function() { + return errors.join('\n'); + }; + + var result = { + pass: errors.length === 0, + message: message() + }; + + return result; + }, + negativeCompare: function(actual) { + var errors = []; + if (actual.selected) { + errors.push('Expected option property "selected" to be falsy'); + } + + if (msie !== 9 && actual.hasAttribute('selected')) { + errors.push('Expected option not to have attribute "selected"'); + } + + var message = function() { + return errors.join('\n'); + }; + + var result = { + pass: errors.length === 0, + message: message() + }; + + return result; + } + }; } }); }); @@ -744,6 +793,41 @@ describe('ngOptions', function() { }); + it('should remove the "selected" attribute from the previous option when the model changes', function() { + scope.values = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}]; + + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'item.label for item in values' + }, true); + + var options = element.find('option'); + expect(options[0]).toBeMarkedAsSelected(); + expect(options[1]).not.toBeMarkedAsSelected(); + expect(options[2]).not.toBeMarkedAsSelected(); + + scope.selected = scope.values[0]; + scope.$digest(); + + expect(options[0]).not.toBeMarkedAsSelected(); + expect(options[1]).toBeMarkedAsSelected(); + expect(options[2]).not.toBeMarkedAsSelected(); + + scope.selected = scope.values[1]; + scope.$digest(); + + expect(options[0]).not.toBeMarkedAsSelected(); + expect(options[1]).not.toBeMarkedAsSelected(); + expect(options[2]).toBeMarkedAsSelected(); + + scope.selected = 'no match'; + scope.$digest(); + + expect(options[0]).toBeMarkedAsSelected(); + expect(options[1]).not.toBeMarkedAsSelected(); + expect(options[2]).not.toBeMarkedAsSelected(); + }); + describe('disableWhen expression', function() { describe('on single select', function() { @@ -1395,6 +1479,26 @@ describe('ngOptions', function() { }); }).not.toThrow(); }); + + it('should remove the "selected" attribute when the model changes', function() { + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'item.label for item in arr track by item.id' + }); + + var options = element.find('option'); + browserTrigger(options[2], 'click'); + + expect(scope.selected).toEqual(scope.arr[1]); + + scope.selected = {}; + scope.$digest(); + + expect(options[0]).toBeMarkedAsSelected(); + expect(options[2]).not.toBeMarkedAsSelected(); + expect(options[2]).not.toBeMarkedAsSelected(); + }); + });