From 27486bd15e70946ece2ba713e4e8654b7f9bddad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Thu, 6 Dec 2018 10:07:55 +0100 Subject: [PATCH] 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 --- src/ng/compile.js | 11 ++++++++++- test/ng/compileSpec.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b7652cf084f8..0bb6e386eddc 100644 --- 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 index 92fc545f3139..83b607d58b31 100644 --- 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);