From 401701909cc2e2bec73a19f843c5d1ac3dea24a1 Mon Sep 17 00:00:00 2001
From: Pol Bonastre <pbr1111@gmail.com>
Date: Fri, 17 Mar 2017 19:00:54 +0100
Subject: [PATCH 1/3] perf(ngOptions): prevent initial options repaiting

Avoid double execution of `updateOptions()` method, which causes a complete repainting of all `<option>` elements.

Fixes angular/angular.js#15801
---
 src/ng/directive/ngOptions.js | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js
index e82d5e49813a..56e1f7d130d4 100644
--- a/src/ng/directive/ngOptions.js
+++ b/src/ng/directive/ngOptions.js
@@ -428,6 +428,9 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
         }
       }
 
+      // The empty option will be compiled and rendered before we first generate the options
+      selectElement.empty();
+
       var providedEmptyOption = !!selectCtrl.emptyOption;
 
       var unknownOption = jqLite(optionTemplate.cloneNode(false));
@@ -449,6 +452,9 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
       if (!multiple) {
 
         selectCtrl.writeValue = function writeNgOptionsValue(value) {
+          if (!options)
+            return;
+
           var selectedOption = options.selectValueMap[selectElement.val()];
           var option = options.getOptionFromViewValue(value);
 
@@ -508,6 +514,9 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
       } else {
 
         selectCtrl.writeValue = function writeNgOptionsMultiple(values) {
+          if (!options)
+            return;
+
           // Only set `<option>.selected` if necessary, in order to prevent some browsers from
           // scrolling to `<option>` elements that are outside the `<select>` element's viewport.
 
@@ -552,13 +561,12 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
 
       if (providedEmptyOption) {
 
-        // we need to remove it before calling selectElement.empty() because otherwise IE will
-        // remove the label from the element. wtf?
-        selectCtrl.emptyOption.remove();
-
         // compile the element since there might be bindings in it
         $compile(selectCtrl.emptyOption)(scope);
 
+        // Ensure that the empty option is always there if it was explicitly provided
+        selectElement.prepend(selectCtrl.emptyOption);
+
         if (selectCtrl.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.
@@ -590,12 +598,6 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
 
       }
 
-      selectElement.empty();
-
-      // We need to do this here to ensure that the options object is defined
-      // when we first hit it in writeNgOptionsValue
-      updateOptions();
-
       // We will re-render the option elements if the option values or labels change
       scope.$watchCollection(ngOptions.getWatchables, updateOptions);
 
@@ -624,7 +626,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
         // the option list in listbox style, i.e. the select is [multiple], or specifies a [size].
         // See https://github.com/angular/angular.js/issues/11314 for more info.
         // This is unfortunately untestable with unit / e2e tests
-        if (option.label !== element.label) {
+        if (isDefined(option.label)) {
           element.label = option.label;
           element.textContent = option.label;
         }
@@ -655,11 +657,6 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
 
         var groupElementMap = {};
 
-        // Ensure that the empty option is always there if it was explicitly provided
-        if (providedEmptyOption) {
-          selectElement.prepend(selectCtrl.emptyOption);
-        }
-
         options.items.forEach(function addOption(option) {
           var groupElement;
 

From 7135eebd79f394071a6ad3a616b7d85c3867f5f3 Mon Sep 17 00:00:00 2001
From: Pol Bonastre <pbr1111@gmail.com>
Date: Fri, 24 Mar 2017 20:55:46 +0100
Subject: [PATCH 2/3] test(ngOptions): ignore comments within a <select>
 element

ngOptions should ignore comments within a `<select>`element and keep only the `<!-- ngIf -->` comment of the empty option.

Ref angular/angular.js#15812
---
 test/ng/directive/ngOptionsSpec.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
index 7a3c4ba492bf..c62bfb25aad3 100644
--- a/test/ng/directive/ngOptionsSpec.js
+++ b/test/ng/directive/ngOptionsSpec.js
@@ -2641,6 +2641,25 @@ describe('ngOptions', function() {
       dealoc(element);
     }));
 
+
+    it('should remove all comments inside the select element except the initial falsy ngIf empty option', function() {
+      scope.values = [
+        {name:'black'},
+        {name:'white'},
+        {name:'red'}
+      ];
+      scope.isBlank = false;
+
+      createSingleSelect('<!-- test comment -->'
+                          + '<option ng-if="isBlank" value="">blank</option>'
+                          + '<!-- test comment 2 -->');
+      scope.$apply('isBlank = true');
+
+      expect(element.find('option')[0].value).toBe('');
+      expect(element.find('option')[0].textContent).toBe('blank');
+      expect(element.find('option')[1].textContent).toBe('black');
+    });
+
   });
 
 

From 8651d9f19450f7cae7f8d00fe8a6b219e96e0233 Mon Sep 17 00:00:00 2001
From: Pol Bonastre <pbr1111@gmail.com>
Date: Fri, 24 Mar 2017 21:18:49 +0100
Subject: [PATCH 3/3] test(ngOptions): ensure options aren't repainted when
 initialize ngOptions

Since https://github.com/angular/angular.js/commit/97b3e003bc60f979fddecd16bc50304b5bef7e53, ngOptions are repainting the options twice when it initializes. This test verifies that the options arent unnecessarily repainted.

Ref angular/angular.js#15812
---
 test/ng/directive/ngOptionsSpec.js | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
index c62bfb25aad3..40b56b241e9b 100644
--- a/test/ng/directive/ngOptionsSpec.js
+++ b/test/ng/directive/ngOptionsSpec.js
@@ -779,6 +779,32 @@ describe('ngOptions', function() {
     expect(options[2]).not.toBeMarkedAsSelected();
   });
 
+
+  it('should render the initial options only one time', function() {
+    scope.values = ['black', 'white', 'red'];
+    // Insert a select element into the DOM without ng-options to suscribe to DOM change events
+    createSelect({
+      'ng-model': 'value'
+    });
+
+    // Add ngOptions as attribute before recompiling the select element
+    element.attr('ng-options', 'value for value in values');
+
+    var repaint = 0;
+    // Detect only when the childNodes count changes
+    var lastElementChildrenCount = -1;
+    element[0].addEventListener('DOMSubtreeModified', function(ev) {
+      var childrenCount = jqLite(ev.target).contents().length;
+      if (childrenCount !== lastElementChildrenCount) {
+        repaint++;
+        lastElementChildrenCount = childrenCount;
+        expect(repaint).toBeLessThan(2);
+      }
+    });
+
+    $compile(element)(scope);
+  });
+
   describe('disableWhen expression', function() {
 
     describe('on single select', function() {