User "Aaron Schulz" changed the status of MediaWiki.r94386. Old Status: new New Status: ok
Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/94386 Commit summary: Use .prop instead of .attr where appropriate * Although jQuery covers for us in most cases (since .prop didn't exist before jQuery 1.6 and many people abused .attr in laziness of doing their own loop and setting the property manually). It's better to know what we're doing and call the function we intend to call. Both because jQuery may decide to stop rerouting common mistakes to .prop and because it makes code more readable. * Aside from switching to prop, for boolean properties also replacing null/undefined with false and 'propname' with true. This is no longer being covered by jQuery when using prop directly (as it shouldn't). When an element is created the HTML specification says that the attribute should be set to it's name (ie. '<foo selected="selected">'), the properties however must remain boolean. * Changing the attribute value for a boolean property (ie. checkbox.setAttribute( 'checked', 'checked' ) does not make the checkbox enabled. All it does is set the attribute. The reason this works with jQuery's attr() is because jQuery calls .prop internally after a bunch of checking inside attr(). -- Reference -- The list of keys that .attr and .removeAttr jQuery is currently (as of jQuery 1.6.1) remapping to use .prop and .removeProp can be found here: https://github.com/jquery/jquery/blob/b22c9046529852c7ce567df13397849e11e2b9cc/src/attributes.js#L425 { tabindex: "tabIndex", readonly: "readOnly", "for": "htmlFor", "class": "className", maxlength: "maxLength", cellspacing: "cellSpacing", cellpadding: "cellPadding", rowspan: "rowSpan", colspan: "colSpan", usemap: "useMap", frameborder: "frameBorder", contenteditable: "contentEditable" } In addition to those, jQuery also maps these boolean properties to .prop when they are passed to .attr: rboolean = /^(?:autofocus|autoplay|async|checked|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped|selected)$/i, (source: https://github.com/jquery/jquery/blob/b22c9046529852c7ce567df13397849e11e2b9cc/src/attributes.js#L9 ) _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
