From 7ceb5f6fcc43d35d1b66c3151ce6a71c60309304 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski?= <m.goleb@gmail.com>
Date: Wed, 21 Sep 2016 14:38:01 +0200
Subject: [PATCH] refactor(jqLite): Don't get/set props when getting/setting
 bool attrs

This is done automatically by browsers in cases where it's needed; the
workaround was only needed for IE<9. The new behavior means boolean attributes
will not be reflected on elements where browsers don't reflect them.

This change aligns jqLite with jQuery 3.

Fixes #14126

BREAKING CHANGE: Previously, all boolean attributes were reflected into
properties in a setter and from a property in a getter, even on elements that
don't treat those attributes in a special way. Now Angular doesn't do it
by itself but relies on browsers to know when to reflect the property. Note that
this browser-level conversions differs between browsers; if you need to change
dynamic state of an element you should modify the property, not the attribute.

See https://jquery.com/upgrade-guide/1.9/#attr-versus-prop- for a more detailed
description about a related change in jQuery 1.9.

To migrate the code follow the example below:

Before:

CSS:
input[checked="checked"] { ... }

JS:
elem1.attr('checked', 'checked');
elem2.attr('checked', false);

After:

CSS:
input:checked { ... }

JS:
elem1.prop('checked', true);
elem2.prop('checked', false);
---
 src/jqLite.js      | 11 +++--------
 test/jqLiteSpec.js | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/src/jqLite.js b/src/jqLite.js
index 15f1bb06f..8da0e213f 100644
--- a/src/jqLite.js
+++ b/src/jqLite.js
@@ -646,17 +646,12 @@ forEach({
     if (BOOLEAN_ATTR[lowercasedName]) {
       if (isDefined(value)) {
         if (value) {
-          element[name] = true;
-          element.setAttribute(name, lowercasedName);
+          element.setAttribute(name, name);
         } else {
-          element[name] = false;
-          element.removeAttribute(lowercasedName);
+          element.removeAttribute(name);
         }
       } else {
-        return (element[name] ||
-                 (element.attributes.getNamedItem(name) || noop).specified)
-               ? lowercasedName
-               : undefined;
+        return element.getAttribute(name) != null ? lowercasedName : undefined;
       }
     } else if (isDefined(value)) {
       element.setAttribute(name, value);
diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js
index 502cfe67c..90247614c 100644
--- a/test/jqLiteSpec.js
+++ b/test/jqLiteSpec.js
@@ -635,6 +635,43 @@ describe('jqLite', function() {
       expect(select.attr('multiple')).toBe('multiple');
     });
 
+    it('should not take properties into account when getting respective boolean attributes', function() {
+      // Use a div and not a select as the latter would itself reflect the multiple attribute
+      // to a property.
+      var div = jqLite('<div>');
+
+      div[0].multiple = true;
+      expect(div.attr('multiple')).toBe(undefined);
+
+      div.attr('multiple', 'multiple');
+      div[0].multiple = false;
+      expect(div.attr('multiple')).toBe('multiple');
+    });
+
+    it('should not set properties when setting respective boolean attributes', function() {
+      // jQuery 2.x has different behavior; skip the test.
+      if (isJQuery2x()) return;
+
+      // Use a div and not a select as the latter would itself reflect the multiple attribute
+      // to a property.
+      var div = jqLite('<div>');
+
+      // Check the initial state.
+      expect(div[0].multiple).toBe(undefined);
+
+      div.attr('multiple', 'multiple');
+      expect(div[0].multiple).toBe(undefined);
+
+      div.attr('multiple', '');
+      expect(div[0].multiple).toBe(undefined);
+
+      div.attr('multiple', false);
+      expect(div[0].multiple).toBe(undefined);
+
+      div.attr('multiple', null);
+      expect(div[0].multiple).toBe(undefined);
+    });
+
     it('should normalize the case of boolean attributes', function() {
       var input = jqLite('<input readonly>');
       expect(input.attr('readonly')).toBe('readonly');