From 9437fc94681337ccde7e6df061898abe88bf2548 Mon Sep 17 00:00:00 2001
From: Peter Bacon Darwin <pete@bacondarwin.com>
Date: Thu, 13 Nov 2014 14:51:56 +0000
Subject: [PATCH 1/3] test(select): refactor option elements expectations to
 use `toEqualOption` matcher

By using a new matcher our tests become less brittle with respect to unimportant
extra attributes.
---
 test/ng/directive/selectSpec.js | 78 +++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 29 deletions(-)

diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
index 7fb1121f4720..42aea7b181dc 100644
--- a/test/ng/directive/selectSpec.js
+++ b/test/ng/directive/selectSpec.js
@@ -38,8 +38,28 @@ describe('select', function() {
         };
 
         return equals(expectedValues, actualValues);
+      },
+
+      toEqualOption: function(value, text, label) {
+        var errors = [];
+        if (this.actual.attr('value') !== value) {
+          errors.push('Expected option value "' + this.actual.attr('value') + '" to equal "' + value + '"');
+        }
+        if (text && this.actual.text() !== text) {
+          errors.push('Expected option value "' + this.actual.attr('value') + '" to equal "' + value + '"');
+        }
+        if (label && this.actual.attr('label') !== label) {
+          errors.push('Expected option value "' + this.actual.attr('value') + '" to equal "' + value + '"');
+        }
+
+        this.message = function() {
+          return errors.join('\n');
+        };
+
+        return errors.length === 0;
       }
     });
+
   });
 
 
@@ -756,9 +776,9 @@ describe('select', function() {
 
         var options = element.find('option');
         expect(options.length).toEqual(3);
-        expect(sortedHtml(options[0])).toEqual('<option value="?"></option>');
-        expect(sortedHtml(options[1])).toEqual('<option value="10">ten</option>');
-        expect(sortedHtml(options[2])).toEqual('<option value="20">twenty</option>');
+        expect(options.eq(0)).toEqualOption('?', '');
+        expect(options.eq(1)).toEqualOption('10', 'ten');
+        expect(options.eq(2)).toEqualOption('20', 'twenty');
       });
 
       it('should preserve value even when reference has changed (single&array)', function() {
@@ -1047,9 +1067,9 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(3);
-      expect(sortedHtml(options[0])).toEqual('<option value="0">A</option>');
-      expect(sortedHtml(options[1])).toEqual('<option value="1">B</option>');
-      expect(sortedHtml(options[2])).toEqual('<option value="2">C</option>');
+      expect(options.eq(0)).toEqualOption('0', 'A');
+      expect(options.eq(1)).toEqualOption('1', 'B');
+      expect(options.eq(2)).toEqualOption('2', 'C');
     });
 
     it('should render zero as a valid display value', function() {
@@ -1062,9 +1082,9 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(3);
-      expect(sortedHtml(options[0])).toEqual('<option value="0">0</option>');
-      expect(sortedHtml(options[1])).toEqual('<option value="1">1</option>');
-      expect(sortedHtml(options[2])).toEqual('<option value="2">2</option>');
+      expect(options.eq(0)).toEqualOption('0', '0');
+      expect(options.eq(1)).toEqualOption('1', '1');
+      expect(options.eq(2)).toEqualOption('2', '2');
     });
 
 
@@ -1081,9 +1101,9 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(3);
-      expect(sortedHtml(options[0])).toEqual('<option value="blue">blue</option>');
-      expect(sortedHtml(options[1])).toEqual('<option value="green">green</option>');
-      expect(sortedHtml(options[2])).toEqual('<option value="red">red</option>');
+      expect(options.eq(0)).toEqualOption('blue', 'blue');
+      expect(options.eq(1)).toEqualOption('green', 'green');
+      expect(options.eq(2)).toEqualOption('red', 'red');
       expect(options[2].selected).toEqual(true);
 
       scope.$apply(function() {
@@ -1103,7 +1123,7 @@ describe('select', function() {
       });
 
       expect(element.find('option').length).toEqual(1); // because we add special empty option
-      expect(sortedHtml(element.find('option')[0])).toEqual('<option value="?"></option>');
+      expect(element.find('option')).toEqualOption('?','');
 
       scope.$apply(function() {
         scope.values.push({name:'A'});
@@ -1111,15 +1131,15 @@ describe('select', function() {
       });
 
       expect(element.find('option').length).toEqual(1);
-      expect(sortedHtml(element.find('option')[0])).toEqual('<option value="0">A</option>');
+      expect(element.find('option')).toEqualOption('0', 'A');
 
       scope.$apply(function() {
         scope.values.push({name:'B'});
       });
 
       expect(element.find('option').length).toEqual(2);
-      expect(sortedHtml(element.find('option')[0])).toEqual('<option value="0">A</option>');
-      expect(sortedHtml(element.find('option')[1])).toEqual('<option value="1">B</option>');
+      expect(element.find('option').eq(0)).toEqualOption('0', 'A');
+      expect(element.find('option').eq(1)).toEqualOption('1', 'B');
     });
 
 
@@ -1138,15 +1158,15 @@ describe('select', function() {
       });
 
       expect(element.find('option').length).toEqual(2);
-      expect(sortedHtml(element.find('option')[0])).toEqual('<option value="0">A</option>');
-      expect(sortedHtml(element.find('option')[1])).toEqual('<option value="1">B</option>');
+      expect(element.find('option').eq(0)).toEqualOption('0', 'A');
+      expect(element.find('option').eq(1)).toEqualOption('1', 'B');
 
       scope.$apply(function() {
         scope.values.pop();
       });
 
       expect(element.find('option').length).toEqual(1);
-      expect(sortedHtml(element.find('option')[0])).toEqual('<option value="0">A</option>');
+      expect(element.find('option')).toEqualOption('0', 'A');
 
       scope.$apply(function() {
         scope.values.pop();
@@ -1198,9 +1218,9 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(3);
-      expect(sortedHtml(options[0])).toEqual('<option value="0">B</option>');
-      expect(sortedHtml(options[1])).toEqual('<option value="1">C</option>');
-      expect(sortedHtml(options[2])).toEqual('<option value="2">D</option>');
+      expect(options.eq(0)).toEqualOption('0', 'B');
+      expect(options.eq(1)).toEqualOption('1', 'C');
+      expect(options.eq(2)).toEqualOption('2', 'D');
     });
 
 
@@ -1244,7 +1264,7 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(1);
-      expect(sortedHtml(options[0])).toEqual('<option value="regularProperty">visible</option>');
+      expect(options.eq(0)).toEqualOption('regularProperty', 'visible');
     });
 
     it('should allow expressions over multiple lines', function() {
@@ -1268,8 +1288,8 @@ describe('select', function() {
 
       var options = element.find('option');
       expect(options.length).toEqual(3);
-      expect(sortedHtml(options[1])).toEqual('<option value="0">2</option>');
-      expect(sortedHtml(options[2])).toEqual('<option value="1">3</option>');
+      expect(options.eq(1)).toEqualOption('0', '2');
+      expect(options.eq(2)).toEqualOption('1', '3');
     });
 
     it('should not update selected property of an option element on digest with no change event',
@@ -1426,8 +1446,8 @@ describe('select', function() {
 
         var options = element.find('option');
         expect(options.length).toEqual(2);
-        expect(sortedHtml(options[0])).toEqual('<option value="0">C</option>');
-        expect(sortedHtml(options[1])).toEqual('<option value="1">B</option>');
+        expect(options.eq(0)).toEqualOption('0', 'C');
+        expect(options.eq(1)).toEqualOption('1', 'B');
       });
 
 
@@ -1447,8 +1467,8 @@ describe('select', function() {
 
         var options = element.find('option');
         expect(options.length).toEqual(2);
-        expect(sortedHtml(options[0])).toEqual('<option value="a">C</option>');
-        expect(sortedHtml(options[1])).toEqual('<option value="b">B</option>');
+        expect(options.eq(0)).toEqualOption('a', 'C');
+        expect(options.eq(1)).toEqualOption('b', 'B');
       });
 
 

From a2170feb94bcc9d7b1019946d1e59d38b83d4850 Mon Sep 17 00:00:00 2001
From: Peter Bacon Darwin <pete@bacondarwin.com>
Date: Thu, 13 Nov 2014 14:51:56 +0000
Subject: [PATCH 2/3] fix(select): ensure the label attribute is updated in
 Internet Explorer

Only changing the `<option>` text value is not enough to trigger a render
change in IE. We need to explicit update the `label` attribute too.

Closes #9621
---
 src/ng/directive/select.js      |  2 ++
 test/ng/directive/selectSpec.js | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
index b957fa623b31..753e0f7c1d37 100644
--- a/src/ng/directive/select.js
+++ b/src/ng/directive/select.js
@@ -623,6 +623,7 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                   updateLabelMap(labelMap, existingOption.label, false);
                   updateLabelMap(labelMap, option.label, true);
                   lastElement.text(existingOption.label = option.label);
+                  msie && lastElement.attr('label', existingOption.label);
                 }
                 if (existingOption.id !== option.id) {
                   lastElement.val(existingOption.id = option.id);
@@ -653,6 +654,7 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                       .prop('selected', option.selected)
                       .attr('selected', option.selected)
                       .text(option.label);
+                  msie && element.attr('label', option.label);
                 }
 
                 existingOptions.push(existingOption = {
diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
index 42aea7b181dc..0781edc76252 100644
--- a/test/ng/directive/selectSpec.js
+++ b/test/ng/directive/selectSpec.js
@@ -1320,6 +1320,25 @@ describe('select', function() {
       expect(scope.selected).toBe(scope.values[0]);
     });
 
+    // bug fix #9621
+    if (msie) {
+      it('should update the label attribute if running in MS IE', function() {
+        // ng-options="value.name for value in values"
+        // ng-model="selected"
+        createSingleSelect();
+
+        scope.$apply(function() {
+          scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
+          scope.selected = scope.values[0];
+        });
+
+        var options = element.find('option');
+        expect(options.eq(0).attr('label')).toEqual('A');
+        expect(options.eq(1).attr('label')).toEqual('B');
+        expect(options.eq(2).attr('label')).toEqual('C');
+      });
+    }
+
     describe('binding', function() {
 
       it('should bind to scope value', function() {

From cface786738161cbc19571bfee5dce7fbb56859f Mon Sep 17 00:00:00 2001
From: Caitlin Potter <caitpotter88@gmail.com>
Date: Thu, 13 Nov 2014 10:45:20 -0500
Subject: [PATCH 3/3] refactor(select): make MSIE behaviour consistent with
 other browsers

---
 src/ng/directive/select.js      |  6 +++---
 test/ng/directive/selectSpec.js | 30 +++++++++++++++---------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
index 753e0f7c1d37..9670c94cb89c 100644
--- a/src/ng/directive/select.js
+++ b/src/ng/directive/select.js
@@ -623,7 +623,7 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                   updateLabelMap(labelMap, existingOption.label, false);
                   updateLabelMap(labelMap, option.label, true);
                   lastElement.text(existingOption.label = option.label);
-                  msie && lastElement.attr('label', existingOption.label);
+                  lastElement.attr('label', existingOption.label);
                 }
                 if (existingOption.id !== option.id) {
                   lastElement.val(existingOption.id = option.id);
@@ -653,8 +653,8 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                       .val(option.id)
                       .prop('selected', option.selected)
                       .attr('selected', option.selected)
-                      .text(option.label);
-                  msie && element.attr('label', option.label);
+                      .text(option.label)
+                      .attr('label', option.label);
                 }
 
                 existingOptions.push(existingOption = {
diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
index 0781edc76252..517001dc931f 100644
--- a/test/ng/directive/selectSpec.js
+++ b/test/ng/directive/selectSpec.js
@@ -1320,24 +1320,24 @@ describe('select', function() {
       expect(scope.selected).toBe(scope.values[0]);
     });
 
-    // bug fix #9621
-    if (msie) {
-      it('should update the label attribute if running in MS IE', function() {
-        // ng-options="value.name for value in values"
-        // ng-model="selected"
-        createSingleSelect();
 
-        scope.$apply(function() {
-          scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
-          scope.selected = scope.values[0];
-        });
+    // bug fix #9621
+    it('should update the label attribute if running in MS IE', function() {
+      // ng-options="value.name for value in values"
+      // ng-model="selected"
+      createSingleSelect();
 
-        var options = element.find('option');
-        expect(options.eq(0).attr('label')).toEqual('A');
-        expect(options.eq(1).attr('label')).toEqual('B');
-        expect(options.eq(2).attr('label')).toEqual('C');
+      scope.$apply(function() {
+        scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
+        scope.selected = scope.values[0];
       });
-    }
+
+      var options = element.find('option');
+      expect(options.eq(0).attr('label')).toEqual('A');
+      expect(options.eq(1).attr('label')).toEqual('B');
+      expect(options.eq(2).attr('label')).toEqual('C');
+    });
+
 
     describe('binding', function() {