fix(compile): properly handle false value for boolean attrs with 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 Closes #16779
angular · Dec 6, 2018 · 09f013a · 09f013a
1 parent de0aad8
commit 09f013a
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 1 deletion.
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -2231,7 +2231,16 @@ 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
+              if (booleanKey && 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);