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 #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);
This commit is contained in:
Michał Gołębiowski
2016-09-21 14:38:01 +02:00
parent 1f16c79396
commit 7ceb5f6fcc
2 changed files with 40 additions and 8 deletions
+3 -8
View File
@@ -646,17 +646,12 @@ forEach({
if (BOOLEAN_ATTR[lowercasedName]) {
if (isDefined(value)) {
if (value) {
element[name] = true;
element.setAttribute(name, lowercasedName);
element.setAttribute(name, name);
} else {
element[name] = false;
element.removeAttribute(lowercasedName);
element.removeAttribute(name);
}
} else {
return (element[name] ||
(element.attributes.getNamedItem(name) || noop).specified)
? lowercasedName
: undefined;
return element.getAttribute(name) != null ? lowercasedName : undefined;
}
} else if (isDefined(value)) {
element.setAttribute(name, value);
+37
View File
@@ -635,6 +635,43 @@ describe('jqLite', function() {
expect(select.attr('multiple')).toBe('multiple');
});
it('should not take properties into account when getting respective boolean attributes', function() {
// Use a div and not a select as the latter would itself reflect the multiple attribute
// to a property.
var div = jqLite('<div>');
div[0].multiple = true;
expect(div.attr('multiple')).toBe(undefined);
div.attr('multiple', 'multiple');
div[0].multiple = false;
expect(div.attr('multiple')).toBe('multiple');
});
it('should not set properties when setting respective boolean attributes', function() {
// jQuery 2.x has different behavior; skip the test.
if (isJQuery2x()) return;
// Use a div and not a select as the latter would itself reflect the multiple attribute
// to a property.
var div = jqLite('<div>');
// Check the initial state.
expect(div[0].multiple).toBe(undefined);
div.attr('multiple', 'multiple');
expect(div[0].multiple).toBe(undefined);
div.attr('multiple', '');
expect(div[0].multiple).toBe(undefined);
div.attr('multiple', false);
expect(div[0].multiple).toBe(undefined);
div.attr('multiple', null);
expect(div[0].multiple).toBe(undefined);
});
it('should normalize the case of boolean attributes', function() {
var input = jqLite('<input readonly>');
expect(input.attr('readonly')).toBe('readonly');