fix(ngOptions): select disabled option when set from model

When a disabled option is set as the ngModel, ngOptions will now select the option
in the select element, which will set select.val() to the option hash value and visually
show the option value / label as selected in the select box. Previously, disabled
options forced the unknown value.
The previous behavior is inconsistent with both default HTML behavior and select with
ngModel but without ngOptions. Both allow disabled values to be selected programmatically.

Common use cases for this behavior include options that where previously valid, but have
been disabled, and cannot be selected again.

This commit removes a duplicate tests, all other tests that previously checked that disabled
options are not set have been adjusted to the ensure the opposite.

Fixes angular#12756

Author: Narretz

src/ng/directive/ngOptions.js

@@ -467,7 +467,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
         selectCtrl.writeValue = function writeNgOptionsValue(value) {
           var option = options.getOptionFromViewValue(value);
 
-          if (option && !option.disabled) {
+          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,
             // most properties are set automatically - except the `selected` attribute, which we
@@ -529,7 +529,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
           if (value) {
             value.forEach(function(item) {
               var option = options.getOptionFromViewValue(item);
-              if (option && !option.disabled) option.element.selected = true;
+              if (option) option.element.selected = true;
             });
           }
         };

test/ng/directive/ngOptionsSpec.js

@@ -751,7 +751,7 @@ describe('ngOptions', function() {
       });
 
 
-      it('should not select disabled options when model changes', function() {
+      it('should select disabled options when model changes', function() {
         scope.options = [
           { name: 'white', value: '#FFFFFF' },
           { name: 'one', value: 1, unavailable: true },
@@ -772,11 +772,14 @@ describe('ngOptions', function() {
         scope.$apply('selected = 1');
         options = element.find('option');
 
-        expect(element.val()).toEqualUnknownValue('?');
-        expect(options.length).toEqual(5);
-        expect(options.eq(0).prop('selected')).toEqual(true);
+        // jQuery returns null for val() when the option is disabled, see
+        // https://bugs.jquery.com/ticket/13097
+        expect(element[0].value).toBe('number:1');
+        expect(options.length).toEqual(4);
+        expect(options.eq(0).prop('selected')).toEqual(false);
+        expect(options.eq(1).prop('selected')).toEqual(true);
         expect(options.eq(2).prop('selected')).toEqual(false);
-        expect(options.eq(4).prop('selected')).toEqual(false);
+        expect(options.eq(3).prop('selected')).toEqual(false);
       });
 
 
@@ -796,11 +799,14 @@ describe('ngOptions', function() {
         scope.$apply('selected = 1');
         var options = element.find('option');
 
-        expect(element.val()).toEqualUnknownValue('?');
-        expect(options.length).toEqual(5);
-        expect(options.eq(0).prop('selected')).toEqual(true);
+        // jQuery returns null for val() when the option is disabled, see
+        // https://bugs.jquery.com/ticket/13097
+        expect(element[0].value).toBe('number:1');
+        expect(options.length).toEqual(4);
+        expect(options.eq(0).prop('selected')).toEqual(false);
+        expect(options.eq(1).prop('selected')).toEqual(true);
         expect(options.eq(2).prop('selected')).toEqual(false);
-        expect(options.eq(4).prop('selected')).toEqual(false);
+        expect(options.eq(3).prop('selected')).toEqual(false);
 
         // Now enable that option
         scope.$apply(function() {
@@ -841,7 +847,7 @@ describe('ngOptions', function() {
       });
 
 
-      it('should not select disabled options when model changes', function() {
+      it('should select disabled options when model changes', function() {
         scope.options = [
           { name: 'a', value: 0 },
           { name: 'b', value: 1, unavailable: true },
@@ -866,14 +872,14 @@ describe('ngOptions', function() {
         scope.$apply('selected = [1,3]');
         options = element.find('option');
         expect(options.eq(0).prop('selected')).toEqual(false);
-        expect(options.eq(1).prop('selected')).toEqual(false);
+        expect(options.eq(1).prop('selected')).toEqual(true);
         expect(options.eq(2).prop('selected')).toEqual(false);
         expect(options.eq(3).prop('selected')).toEqual(true);
 
         // Now only select the disabled option
         scope.$apply('selected = [1]');
         expect(options.eq(0).prop('selected')).toEqual(false);
-        expect(options.eq(1).prop('selected')).toEqual(false);
+        expect(options.eq(1).prop('selected')).toEqual(true);
         expect(options.eq(2).prop('selected')).toEqual(false);
         expect(options.eq(3).prop('selected')).toEqual(false);
       });
@@ -897,7 +903,7 @@ describe('ngOptions', function() {
         var options = element.find('option');
 
         expect(options.eq(0).prop('selected')).toEqual(false);
-        expect(options.eq(1).prop('selected')).toEqual(false);
+        expect(options.eq(1).prop('selected')).toEqual(true);
         expect(options.eq(2).prop('selected')).toEqual(false);
         expect(options.eq(3).prop('selected')).toEqual(false);
 
@@ -2515,37 +2521,6 @@ describe('ngOptions', function() {
       expect(element.find('option')[1].selected).toBeTruthy();
     });
 
-    it('should not write disabled selections from model', function() {
-      scope.selected = [30];
-      scope.options = [
-        { name: 'white', value: '#FFFFFF' },
-        { name: 'one', value: 1, unavailable: true },
-        { name: 'notTrue', value: false },
-        { name: 'thirty', value: 30, unavailable: false }
-      ];
-      createSelect({
-        'ng-options': 'o.value as o.name disable when o.unavailable for o in options',
-        'ng-model': 'selected',
-        'multiple': true
-      });
-
-      var options = element.find('option');
-
-      expect(options.eq(0).prop('selected')).toEqual(false);
-      expect(options.eq(1).prop('selected')).toEqual(false);
-      expect(options.eq(2).prop('selected')).toEqual(false);
-      expect(options.eq(3).prop('selected')).toEqual(true);
-
-      scope.$apply(function() {
-        scope.selected.push(1);
-      });
-
-      expect(options.eq(0).prop('selected')).toEqual(false);
-      expect(options.eq(1).prop('selected')).toEqual(false);
-      expect(options.eq(2).prop('selected')).toEqual(false);
-      expect(options.eq(3).prop('selected')).toEqual(true);
-    });
-
 
     it('should update model on change', function() {
       createMultiSelect();