Permalink
Comparing changes
Open a pull request
- 10 commits
- 2 files changed
- 1 commit comment
- 1 contributor
Commits on Oct 06, 2016
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 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 #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");
This change aligns jqLite with jQuery. Ref #15126 BREAKING CHANGE: Before, using the `attr` method with an empty string as a value would remove the boolean attribute. Now it sets it to its lowercase name as was happening for every non-empty string so far. The only two values that remove the boolean attribute are now null & false, just like in jQuery. To migrate the code follow the example below: Before: elem.attr(booleanAttrName, ''); After: elem.attr(booleanAttrName, false); or: elem.attr(booleanAttrName, null);
The attr method was refactored to be divided into setter & getter parts and to handle boolean attributes in each one of them separately instead of dividing into boolean & non-boolean ones and then handling setter & getter in both of them. This is because handling boolean & non-boolean attributes has common parts; in particular handling of the `null` value or using getAttribute to get the value in the getter.
Unified
Split
Showing
with
122 additions
and 29 deletions.
- +28 −28 src/jqLite.js
- +94 −1 test/jqLiteSpec.js
| @@ -56,7 +56,7 @@ | ||
| * - [`after()`](http://api.jquery.com/after/) | ||
| * - [`append()`](http://api.jquery.com/append/) | ||
| * - [`attr()`](http://api.jquery.com/attr/) - Does not support functions as parameters | ||
| * - [`bind()`](http://api.jquery.com/bind/) - Does not support namespaces, selectors or eventData | ||
| * - [`bind()`](http://api.jquery.com/bind/) (_deprecated_ - to be removed in 1.7.0, use [`on()`](http://api.jquery.com/on/)) - Does not support namespaces, selectors or eventData | ||
| * - [`children()`](http://api.jquery.com/children/) - Does not support selectors | ||
| * - [`clone()`](http://api.jquery.com/clone/) | ||
| * - [`contents()`](http://api.jquery.com/contents/) | ||
| @@ -78,14 +78,14 @@ | ||
| * - [`prop()`](http://api.jquery.com/prop/) | ||
| * - [`ready()`](http://api.jquery.com/ready/) | ||
| * - [`remove()`](http://api.jquery.com/remove/) | ||
| * - [`removeAttr()`](http://api.jquery.com/removeAttr/) | ||
| * - [`removeAttr()`](http://api.jquery.com/removeAttr/) - Does not support multiple attributes | ||
| * - [`removeClass()`](http://api.jquery.com/removeClass/) - Does not support a function as first argument | ||
| * - [`removeData()`](http://api.jquery.com/removeData/) | ||
| * - [`replaceWith()`](http://api.jquery.com/replaceWith/) | ||
| * - [`text()`](http://api.jquery.com/text/) | ||
| * - [`toggleClass()`](http://api.jquery.com/toggleClass/) - Does not support a function as first argument | ||
| * - [`triggerHandler()`](http://api.jquery.com/triggerHandler/) - Passes a dummy event object to handlers | ||
| * - [`unbind()`](http://api.jquery.com/unbind/) - Does not support namespaces or event object as parameter | ||
| * - [`unbind()`](http://api.jquery.com/unbind/) (_deprecated_ - to be removed in 1.7.0, use [`off()`](http://api.jquery.com/off/)) - Does not support namespaces or event object as parameter | ||
| * - [`val()`](http://api.jquery.com/val/) | ||
| * - [`wrap()`](http://api.jquery.com/wrap/) | ||
| * | ||
| @@ -638,33 +638,33 @@ forEach({ | ||
| }, | ||
|
|
||
| attr: function(element, name, value) { | ||
| var ret; | ||
| var nodeType = element.nodeType; | ||
| if (nodeType === NODE_TYPE_TEXT || nodeType === NODE_TYPE_ATTRIBUTE || nodeType === NODE_TYPE_COMMENT) { | ||
| if (nodeType === NODE_TYPE_TEXT || nodeType === NODE_TYPE_ATTRIBUTE || nodeType === NODE_TYPE_COMMENT || | ||
| !element.getAttribute) { | ||
| return; | ||
| } | ||
|
|
||
| var lowercasedName = lowercase(name); | ||
| if (BOOLEAN_ATTR[lowercasedName]) { | ||
| if (isDefined(value)) { | ||
| if (value) { | ||
| element[name] = true; | ||
| element.setAttribute(name, lowercasedName); | ||
| } else { | ||
| element[name] = false; | ||
| element.removeAttribute(lowercasedName); | ||
| } | ||
| var isBooleanAttr = BOOLEAN_ATTR[lowercasedName]; | ||
|
|
||
| if (isDefined(value)) { | ||
| // setter | ||
|
|
||
| if (value === null || (value === false && isBooleanAttr)) { | ||
| element.removeAttribute(name); | ||
| } else { | ||
| return (element[name] || | ||
| (element.attributes.getNamedItem(name) || noop).specified) | ||
| ? lowercasedName | ||
| : undefined; | ||
| element.setAttribute(name, isBooleanAttr ? lowercasedName : value); | ||
| } | ||
| } else if (isDefined(value)) { | ||
| element.setAttribute(name, value); | ||
| } else if (element.getAttribute) { | ||
| // the extra argument "2" is to get the right thing for a.href in IE, see jQuery code | ||
| // some elements (e.g. Document) don't have get attribute, so return undefined | ||
| var ret = element.getAttribute(name, 2); | ||
| // normalize non-existing attributes to undefined (as jQuery) | ||
| } else { | ||
| // getter | ||
|
|
||
| ret = element.getAttribute(name); | ||
|
|
||
| if (isBooleanAttr && ret !== null) { | ||
| ret = lowercasedName; | ||
| } | ||
| // Normalize non-existing attributes to undefined (as jQuery). | ||
| return ret === null ? undefined : ret; | ||
| } | ||
| }, | ||
| @@ -1061,12 +1061,12 @@ forEach({ | ||
| } | ||
| return isDefined(value) ? value : this; | ||
| }; | ||
|
|
||
| // bind legacy bind/unbind to on/off | ||
| JQLite.prototype.bind = JQLite.prototype.on; | ||
| JQLite.prototype.unbind = JQLite.prototype.off; | ||
| }); | ||
|
|
||
| // bind legacy bind/unbind to on/off | ||
| JQLite.prototype.bind = JQLite.prototype.on; | ||
| JQLite.prototype.unbind = JQLite.prototype.off; | ||
|
|
||
|
|
||
| // Provider for private $$jqLite service | ||
| /** @this */ | ||
| @@ -598,7 +598,7 @@ describe('jqLite', function() { | ||
|
|
||
|
|
||
| describe('attr', function() { | ||
| it('should read write and remove attr', function() { | ||
| it('should read, write and remove attr', function() { | ||
| var selector = jqLite([a, b]); | ||
|
|
||
| expect(selector.attr('prop', 'value')).toEqual(selector); | ||
| @@ -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'); | ||
| @@ -684,6 +721,47 @@ describe('jqLite', function() { | ||
| expect(comment.attr('some-attribute','somevalue')).toEqual(comment); | ||
| expect(comment.attr('some-attribute')).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should remove the attribute for a null value', function() { | ||
| var elm = jqLite('<div attribute="value">a</div>'); | ||
| elm.attr('attribute', null); | ||
| expect(elm[0].hasAttribute('attribute')).toBe(false); | ||
| }); | ||
|
|
||
| it('should not remove the attribute for an empty string as a value', function() { | ||
| var elm = jqLite('<div attribute="value">a</div>'); | ||
| elm.attr('attribute', ''); | ||
| expect(elm[0].getAttribute('attribute')).toBe(''); | ||
| }); | ||
|
|
||
| it('should remove the boolean attribute for a false value', function() { | ||
| var elm = jqLite('<select multiple>'); | ||
| elm.attr('multiple', false); | ||
| expect(elm[0].hasAttribute('multiple')).toBe(false); | ||
| }); | ||
|
|
||
| it('should remove the boolean attribute for a null value', function() { | ||
| var elm = jqLite('<select multiple>'); | ||
| elm.attr('multiple', null); | ||
| expect(elm[0].hasAttribute('multiple')).toBe(false); | ||
| }); | ||
|
|
||
| it('should not remove the boolean attribute for an empty string as a value', function() { | ||
| var elm = jqLite('<select multiple>'); | ||
| elm.attr('multiple', ''); | ||
| expect(elm[0].getAttribute('multiple')).toBe('multiple'); | ||
| }); | ||
|
|
||
| it('should not fail on elements without the getAttribute method', function() { | ||
| forEach([window, document], function(node) { | ||
| expect(function() { | ||
| var elem = jqLite(node); | ||
| elem.attr('foo'); | ||
| elem.attr('bar', 'baz'); | ||
| elem.attr('bar'); | ||
| }).not.toThrow(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| @@ -2256,4 +2334,19 @@ describe('jqLite', function() { | ||
| expect(onLoadCallback).toHaveBeenCalledOnce(); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| describe('bind/unbind', function() { | ||
| if (!_jqLiteMode) return; | ||
|
|
||
| it('should alias bind() to on()', function() { | ||
| var element = jqLite(a); | ||
| expect(element.bind).toBe(element.on); | ||
| }); | ||
|
|
||
| it('should alias unbind() to off()', function() { | ||
| var element = jqLite(a); | ||
| expect(element.unbind).toBe(element.off); | ||
| }); | ||
| }); | ||
| }); | ||
Showing you all comments on commits in this comparison.
This comment has been minimized.
This comment has been minimized.
|
Being a |