Skip to content

Commit 4e36245

Browse files
committed
feat(jqLite): remove the attribute for .attr(attribute, null)
This change aligns jqLite with jQuery. Also, the extra `2` second parameter to `setAttribute` has been removed; it was only needed for IE<9 and latest jQuery doesn't pass it either. Ref angular#15126 BREAKING CHANGE: Invoking `elem.attr(attributeName, null)` would set the `attributeName` atribute value to a string `"null"`, now it removes the attribute instead. To migrate the code follow the example below: Before: elem.attr(attributeName, null); After: elem.attr(attributeName, "null");
1 parent 7ceb5f6 commit 4e36245

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

src/jqLite.js

+6-4
Original file line numberDiff line numberDiff line change
@@ -654,11 +654,13 @@ forEach({
654654
return element.getAttribute(name) != null ? lowercasedName : undefined;
655655
}
656656
} else if (isDefined(value)) {
657-
element.setAttribute(name, value);
657+
if (value === null) {
658+
element.removeAttribute(name);
659+
} else {
660+
element.setAttribute(name, value);
661+
}
658662
} else if (element.getAttribute) {
659-
// the extra argument "2" is to get the right thing for a.href in IE, see jQuery code
660-
// some elements (e.g. Document) don't have get attribute, so return undefined
661-
var ret = element.getAttribute(name, 2);
663+
var ret = element.getAttribute(name);
662664
// normalize non-existing attributes to undefined (as jQuery)
663665
return ret === null ? undefined : ret;
664666
}

test/jqLiteSpec.js

+12
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,18 @@ describe('jqLite', function() {
721721
expect(comment.attr('some-attribute','somevalue')).toEqual(comment);
722722
expect(comment.attr('some-attribute')).toBeUndefined();
723723
});
724+
725+
it('should remove the attribute for a null value', function() {
726+
var elm = jqLite('<div attribute="value">a</div>');
727+
elm.attr('attribute', null);
728+
expect(elm[0].hasAttribute('attribute')).toBe(false);
729+
});
730+
731+
it('should not remove the attribute for an empty string as a value', function() {
732+
var elm = jqLite('<div attribute="value">a</div>');
733+
elm.attr('attribute', '');
734+
expect(elm[0].getAttribute('attribute')).toBe('');
735+
});
724736
});
725737

726738

0 commit comments

Comments
 (0)