Skip to content

Commit b1e6a98

Browse files
committed
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 it's only happening for the elements where browsers do the conversion automatically. 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 83b99b7 commit b1e6a98

File tree

2 files changed

+50
-8
lines changed

2 files changed

+50
-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

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

638+
it('should set respective properties when getting/setting boolean attributes', function() {
639+
var select = jqLite('<select>');
640+
641+
// Check initial state.
642+
expect(select.prop('multiple')).toBe(false);
643+
644+
select.attr('multiple', 'truthy');
645+
expect(select.prop('multiple')).toBe(true);
646+
647+
select.attr('multiple', null);
648+
expect(select.prop('multiple')).toBe(false);
649+
});
650+
651+
it('should not take properties into account when getting respective boolean attributes', function() {
652+
// Use a div and not a select as the latter would itself reflect the multiple attribute
653+
// to a property.
654+
var div = jqLite('<div>');
655+
656+
div[0].multiple = true;
657+
expect(div.attr('multiple')).toBe(undefined);
658+
659+
div.attr('multiple', 'multiple');
660+
div[0].multiple = false;
661+
expect(div.attr('multiple')).toBe('multiple');
662+
});
663+
664+
it('should not set properties when settting respective boolean attributes', function() {
665+
// Use a div and not a select as the latter would itself reflect the multiple attribute
666+
// to a property.
667+
var div = jqLite('<div>');
668+
669+
// Check the initial state.
670+
expect(div[0].multiple).toBe(undefined);
671+
672+
div.attr('multiple', 'multiple');
673+
expect(div[0].multiple).toBe(undefined);
674+
675+
div.attr('multiple', '');
676+
expect(div[0].multiple).toBe(undefined);
677+
678+
div.attr('multiple', false);
679+
expect(div[0].multiple).toBe(undefined);
680+
681+
div.attr('multiple', null);
682+
expect(div[0].multiple).toBe(undefined);
683+
});
684+
638685
it('should normalize the case of boolean attributes', function() {
639686
var input = jqLite('<input readonly>');
640687
expect(input.attr('readonly')).toBe('readonly');

0 commit comments

Comments
 (0)