fix(ngOptions): don't add comment nodes as empty options

When the "empty option" element contains a transclusion directive, the result of the compilation always includes a comment node. Since we are adding / removing the "selected" attribute on the empty option, we need to make sure it's an actual element.

To solve this, we take advantage of the fact the each option element has an option directive that tries to register the option with the selectController. With ngOptions, this registerOption function is normally noop'd since it's not possible to add dynamic options. Now if the result of the empty option compilation is a comment, we re-define the function so that it catches empty options when they are actually linked / rendered.

Closes #15454
Closes #15456

Author: Narretz

src/ng/directive/ngOptions.js

@@ -429,6 +429,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
       }
 
       var providedEmptyOption = !!emptyOption;
+      var emptyOptionRendered = false;
 
       var unknownOption = jqLite(optionTemplate.cloneNode(false));
       unknownOption.val('?');
@@ -445,14 +446,16 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
           selectElement.prepend(emptyOption);
         }
         selectElement.val('');
-        emptyOption.prop('selected', true); // needed for IE
-        emptyOption.attr('selected', true);
+        if (emptyOptionRendered) {
+          emptyOption.prop('selected', true); // needed for IE
+          emptyOption.attr('selected', true);
+        }
       };
 
       var removeEmptyOption = function() {
         if (!providedEmptyOption) {
           emptyOption.remove();
-        } else {
+        } else if (emptyOptionRendered) {
           emptyOption.removeAttr('selected');
         }
       };
@@ -587,9 +590,35 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
         // compile the element since there might be bindings in it
         $compile(emptyOption)(scope);
 
-        // remove the class, which is added automatically because we recompile the element and it
-        // becomes the compilation root
-        emptyOption.removeClass('ng-scope');
+        if (emptyOption[0].nodeType === NODE_TYPE_COMMENT) {
+          // This means the empty option has currently no actual DOM node, probably because
+          // it has been modified by a transclusion directive.
+
+          emptyOptionRendered = false;
+
+          // Redefine the registerOption function, which will catch
+          // options that are added by ngIf etc. (rendering of the node is async because of
+          // lazy transclusion)
+          selectCtrl.registerOption = function(optionScope, optionEl) {
+            if (optionEl.val() === '') {
+              emptyOptionRendered = true;
+              emptyOption = optionEl;
+              emptyOption.removeClass('ng-scope');
+              // This ensures the new empty option is selected if previously no option was selected
+              ngModelCtrl.$render();
+
+              optionEl.on('$destroy', function() {
+                emptyOption = undefined;
+                emptyOptionRendered = false;
+              });
+            }
+          };
+
+        } else {
+          emptyOption.removeClass('ng-scope');
+          emptyOptionRendered = true;
+        }
+
       } else {
         emptyOption = jqLite(optionTemplate.cloneNode(false));
       }

test/ng/directive/ngOptionsSpec.js

@@ -2558,6 +2558,95 @@ describe('ngOptions', function() {
     });
 
 
+    it('should select the correct option after linking when the ngIf expression is initially falsy', function() {
+      scope.values = [
+        {name:'black'},
+        {name:'white'},
+        {name:'red'}
+      ];
+      scope.selected = scope.values[2];
+
+      expect(function() {
+        createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
+        scope.$apply();
+      }).not.toThrow();
+
+      expect(element.find('option')[2]).toBeMarkedAsSelected();
+      expect(linkLog).toEqual(['linkNgOptions']);
+    });
+
+
+    it('should add / remove the "selected" attribute on empty option which has an initially falsy ngIf expression', function() {
+      scope.values = [
+        {name:'black'},
+        {name:'white'},
+        {name:'red'}
+      ];
+      scope.selected = scope.values[2];
+
+      createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
+      scope.$apply();
+
+      expect(element.find('option')[2]).toBeMarkedAsSelected();
+
+      scope.$apply('isBlank = true');
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0]).not.toBeMarkedAsSelected();
+
+      scope.$apply('selected = null');
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0]).toBeMarkedAsSelected();
+
+      scope.selected = scope.values[1];
+      scope.$apply();
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0]).not.toBeMarkedAsSelected();
+      expect(element.find('option')[2]).toBeMarkedAsSelected();
+    });
+
+
+    it('should add / remove the "selected" attribute on empty option which has an initially truthy ngIf expression when no option is selected', function() {
+      scope.values = [
+        {name:'black'},
+        {name:'white'},
+        {name:'red'}
+      ];
+      scope.isBlank = true;
+
+      createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
+      scope.$apply();
+
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0]).toBeMarkedAsSelected();
+
+      scope.selected = scope.values[2];
+      scope.$apply();
+      expect(element.find('option')[0]).not.toBeMarkedAsSelected();
+      expect(element.find('option')[3]).toBeMarkedAsSelected();
+    });
+
+
+    it('should add the "selected" attribute on empty option which has an initially falsy ngIf expression when no option is selected', function() {
+      scope.values = [
+        {name:'black'},
+        {name:'white'},
+        {name:'red'}
+      ];
+
+      createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
+      scope.$apply();
+
+      expect(element.find('option')[0]).not.toBeMarkedAsSelected();
+
+      scope.isBlank = true;
+      scope.$apply();
+
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0]).toBeMarkedAsSelected();
+      expect(element.find('option')[1]).not.toBeMarkedAsSelected();
+    });
+
+
     it('should not throw with a directive that replaces', inject(function($templateCache, $httpBackend) {
       $templateCache.put('select_template.html', '<select ng-options="option as option for option in selectable_options"> <option value="">This is a test</option> </select>');