fix(compile): properly handle the false value for boolean attrs wit…

…h jQuery jQuery skips special boolean attrs treatment in XML nodes for historical reasons and hence AngularJS cannot freely call `.attr(attrName, false) with such attributes. To avoid issues in XHTML, call `removeAttr` in such cases instead. Ref jquery/jquery#4249 Fixes #16778
angular · Dec 1, 2018 · 832222e · 832222e
1 parent a5a98d3
commit 832222e
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 2 deletions.
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -2197,7 +2197,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
             booleanKey = getBooleanAttrName(node, key),
             aliasedKey = getAliasedAttrName(key),
             observer = key,
-            nodeName;
+            nodeName, lowercasedName, isBooleanAttr;
 
         if (booleanKey) {
           this.$$element.prop(key, value);
@@ -2231,7 +2231,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
             this.$$element.removeAttr(attrName);
           } else {
             if (SIMPLE_ATTR_NAME.test(attrName)) {
-              this.$$element.attr(attrName, value);
+              // jQuery skips special boolean attrs treatment in XML nodes for
+              // historical reasons and hence AngularJS cannot freely call
+              // `.attr(attrName, false) with such attributes. To avoid issues
+              // in XHTML, call `removeAttr` in such cases instead.
+              // See https://github.com/jquery/jquery/issues/4249
+              lowercasedName = lowercase(attrName);
+              isBooleanAttr = BOOLEAN_ATTR[lowercasedName];
+              if (isBooleanAttr && value === false) {
+                this.$$element.removeAttr(attrName);
+              } else {
+                this.$$element.attr(attrName, value);
+              }
             } else {
               setSpecialAttr(this.$$element[0], attrName, value);
             }

diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -4222,6 +4222,40 @@ describe('$compile', function() {
         expect(element.attr('ng-my-attr')).toBeUndefined();
       });
 
+      it('should set the value to lowercased keys for boolean attrs', function() {
+        attr.$set('disabled', 'value');
+        expect(element.attr('disabled')).toEqual('disabled');
+
+        element.removeAttr('disabled');
+
+        attr.$set('dISaBlEd', 'VaLuE');
+        expect(element.attr('disabled')).toEqual('disabled');
+      });
+
+      it('should call removeAttr for boolean attrs when value is `false`', function() {
+        attr.$set('disabled', 'value');
+
+        spyOn(jqLite.prototype, 'attr').and.callThrough();
+        spyOn(jqLite.prototype, 'removeAttr').and.callThrough();
+
+        attr.$set('disabled', false);
+
+        expect(element.attr).not.toHaveBeenCalled();
+        expect(element.removeAttr).toHaveBeenCalledWith('disabled');
+        expect(element.attr('disabled')).toEqual(undefined);
+
+        attr.$set('disabled', 'value');
+
+        element.attr.calls.reset();
+        element.removeAttr.calls.reset();
+
+        attr.$set('dISaBlEd', false);
+
+        expect(element.attr).not.toHaveBeenCalled();
+        expect(element.removeAttr).toHaveBeenCalledWith('disabled');
+        expect(element.attr('disabled')).toEqual(undefined);
+      });
+
 
       it('should not set DOM element attr if writeAttr false', function() {
         attr.$set('test', 'value', false);