Skip to content

Commit 1f15902

Browse files
mgolellimist
authored andcommitted
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 angular#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);
1 parent 9282a78 commit 1f15902

File tree

2 files changed

+40
-8
lines changed

2 files changed

+40
-8
lines changed

src/jqLite.js

+3-8
Original file line numberDiff line numberDiff line change
@@ -646,17 +646,12 @@ forEach({
646646
if (BOOLEAN_ATTR[lowercasedName]) {
647647
if (isDefined(value)) {
648648
if (value) {
649-
element[name] = true;
650-
element.setAttribute(name, lowercasedName);
649+
element.setAttribute(name, name);
651650
} else {
652-
element[name] = false;
653-
element.removeAttribute(lowercasedName);
651+
element.removeAttribute(name);
654652
}
655653
} else {
656-
return (element[name] ||
657-
(element.attributes.getNamedItem(name) || noop).specified)
658-
? lowercasedName
659-
: undefined;
654+
return element.getAttribute(name) != null ? lowercasedName : undefined;
660655
}
661656
} else if (isDefined(value)) {
662657
element.setAttribute(name, value);

test/jqLiteSpec.js

+37
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,43 @@ describe('jqLite', function() {
635635
expect(select.attr('multiple')).toBe('multiple');
636636
});
637637

638+
it('should not take properties into account when getting respective boolean attributes', function() {
639+
// Use a div and not a select as the latter would itself reflect the multiple attribute
640+
// to a property.
641+
var div = jqLite('<div>');
642+
643+
div[0].multiple = true;
644+
expect(div.attr('multiple')).toBe(undefined);
645+
646+
div.attr('multiple', 'multiple');
647+
div[0].multiple = false;
648+
expect(div.attr('multiple')).toBe('multiple');
649+
});
650+
651+
it('should not set properties when setting respective boolean attributes', function() {
652+
// jQuery 2.x has different behavior; skip the test.
653+
if (isJQuery2x()) return;
654+
655+
// Use a div and not a select as the latter would itself reflect the multiple attribute
656+
// to a property.
657+
var div = jqLite('<div>');
658+
659+
// Check the initial state.
660+
expect(div[0].multiple).toBe(undefined);
661+
662+
div.attr('multiple', 'multiple');
663+
expect(div[0].multiple).toBe(undefined);
664+
665+
div.attr('multiple', '');
666+
expect(div[0].multiple).toBe(undefined);
667+
668+
div.attr('multiple', false);
669+
expect(div[0].multiple).toBe(undefined);
670+
671+
div.attr('multiple', null);
672+
expect(div[0].multiple).toBe(undefined);
673+
});
674+
638675
it('should normalize the case of boolean attributes', function() {
639676
var input = jqLite('<input readonly>');
640677
expect(input.attr('readonly')).toBe('readonly');

0 commit comments

Comments
 (0)