New jQuery announced!
in [JavaScript]
|
From: David Mark on 7 Dec 2009 11:59 But it has the same old attr method. :( attr: function( elem, name, value ) { // don't set attributes on text and comment nodes Don't pass them! Put that in the docs. :) if (!elem || elem.nodeType == 3 || elem.nodeType == 8) { return undefined; } Don't pass null, 0, '', undefined, etc. for elem either. What would be the point of this, other than to make it harder to find bugs? if ( name in jQuery.fn && name !== "attr" ) { return jQuery(elem)[name](value); } It's the do-everything function. ;) var notxml = elem.nodeType !== 1 || !jQuery.isXMLDoc( elem ), // Whether we are setting (or getting) set = value !== undefined; They still seem to think this method is appropriate for XML. Why not call get/setAttribute on XML nodes? That's all this ends up doing. // Try to normalize/fix the name name = notxml && jQuery.props[ name ] || name; Normalize/fix? And jQuery.props is still a long way from complete:- jQuery.props = { "for": "htmlFor", "class": "className", readonly: "readOnly", maxlength: "maxLength", cellspacing: "cellSpacing", rowspan: "rowSpan", colspan: "colSpan", tabindex: "tabIndex", usemap: "useMap", frameborder: "frameBorder" }; How many years does it take for a million code monkeys to come up with the list of attributes that have camel-case property names? More than three apparently. // Only do all the following if this is a node (faster for style) What?! if ( elem.nodeType === 1 ) { // These attributes require special treatment var special = /href|src|style/.test( name ); What sort of "special" treatment? How are href, src and style related? // Safari mis-reports the default selected property of a hidden option // Accessing the parent's selectedIndex property fixes it if ( name == "selected" && elem.parentNode ) { elem.parentNode.selectedIndex; } Mystical incantation. // If applicable, access the attribute via the DOM 0 way Read its property? if ( name in elem && notxml && !special ) { So, if it is - in - the elem, elem is not an XML node and it is not href, src or style, get or set the property. if ( set ) { // We can't allow the type property to be changed (since it causes problems in IE) if ( name == "type" && /(button|input)/i.test(elem.nodeName) && elem.parentNode ) { throw "type property can't be changed"; } Misguided waste of space. elem[ name ] = value; } // browsers index elements by id/name on forms, give priority to attributes. if( jQuery.nodeName( elem, "form" ) && elem.getAttributeNode (name) ) { return elem.getAttributeNode( name ).nodeValue; } // elem.tabIndex doesn't always return the correct value when it hasn't been explicitly set // http://fluidproject.org/blog/2008/01/09/getting-setting-and-removing-tabindex-values-with-javascript/ That article is very confused. :) if ( name == "tabIndex" ) { var attributeNode = elem.getAttributeNode( "tabIndex" ); return attributeNode && attributeNode.specified ? attributeNode.value That's a string. :( : /(button|input|object|select|textarea)/i.test(elem.nodeName) ? 0 That's a number. : /^(a|area)$/i.test(elem.nodeName) && elem.href ? 0 : undefined; } So, tabindex is treated very oddly, returning a string, number or undefined. This attribute is singled out because that article singled it out. This is programming by observation of misinterpreted observations. :( return elem[ name ]; } Then it switches gears to attributes. if ( !jQuery.support.style && notxml && name == "style" ) { if ( set ) { elem.style.cssText = "" + value; What sort of value would this be that it would make sense to convert it to a string? } return elem.style.cssText; } if ( set ) { // convert the value to a string (all browsers do this but IE) see #1070 LOL. I'll pass on #1070. They seem to think all IE's are the same. Of course, IE8 varies wildly here depending on the mode. elem.setAttribute( name, "" + value ); But they already "fixed" the name (converted to a property name):- // Try to normalize/fix the name name = notxml && jQuery.props[ name ] || name; This doesn't make any sense. By coincidence, the - in - check above keeps some properties out of here. One that would fall through (in some browsers, e.g. FF) is "onclick". Passing a function like this:- attr(el, 'onclick', function() { ... }); Would set an odd attribute indeed, considering what Function.prototype.toString does. Of course, if the property existed previously, the previous fork would apply and the method would seem to work. :) } var attr = !jQuery.support.hrefNormalized && notxml && special // Some attributes require a special call on IE More than they know. ;) ? elem.getAttribute( name, 2 ) : elem.getAttribute( name ); This will vary across IE versions and modes. http://www.cinsoft.net/attributes.html // Non-existent attributes return null, we normalize to undefined They don't in IE (except IE8 standards mode). return attr === null ? undefined : attr; } // elem is actually elem.style ... set the style // Using attr for specific style information is now deprecated. Use style insead. return jQuery.style(elem, name, value); } I thought they were going to re-factor this for 2010? I thought they would install at least some versions of IE for testing as well. Maybe next year. :( So they still have trouble reading documents. Odd handicap for a DOM library. And how many times have they been told about these problems? They did like the event detection bit. I don't care for the copy and paste implementation though. // Technique from Juriy Zaytsev I guess they didn't read the article. :( // http://thinkweb2.com/projects/prototype/detecting-event-support-without-browser-sniffing/ var eventSupported = function( eventName ) { var el = document.createElement("div"); eventName = "on" + eventName; var isSupported = (eventName in el); Spotty inference. Keeps IE out of the following:- if ( !isSupported ) { el.setAttribute(eventName, "return;"); isSupported = typeof el[eventName] === "function"; Won't work in IE6/7 or 8 in compatibility mode. But by coincidence, those never get here. Standard IE8 should get here, but relies on the weaker inference above. } el = null; return isSupported; };
From: David Mark on 7 Dec 2009 16:59 On Dec 7, 11:59 am, David Mark <dmark.cins...(a)gmail.com> wrote: > But it has the same old attr method. :( And a new removeAttr "companion" method! removeAttr = function(el, name ) { attr( el, name, "" ); if (el.nodeType == 1) { el.removeAttribute( name ); } }; Keeping with the confusion about attributes/properties, this also tries to do both (poorly). Basically, this sets a property or an attribute to "" (creating one if it does not exist). Then it tries to remove the attribute. The first line attempts to set a property in IE. So this, for example, will throw a wonderful exception in IE < 8 or IE8 in compatibility mode:- removeAttr(el, 'colspan'); // Boom I added a jQuery set to the attribute tests and found that, in addition to all manner of inconsistencies and errors in IE, FF throws an exception on using attr to set DOM0 event handler properties (due to the aforementioned Function to string conversion). They've really got the DOM bumps smoothed over now. ;) After three years of testing and discussion, how can these bugs exist so deep in the core? The only answer is that the authors and community have failed to live up to their press clippings. The revered unit tests are obviously flawed to the point of being useless (not surprising as they were written by the same people who wrote the code). Same for the docs. You just can't test or document what you don't understand. But when things go wrong, you can always blame the browsers, users, etc. Those heroic Ninjas are doing the best they can. ;)
From: Matt Kruse on 7 Dec 2009 17:33 On Dec 7, 3:59 pm, David Mark <dmark.cins...(a)gmail.com> wrote: > removeAttr(el, 'colspan'); // Boom Why would you do this, other than to break things? > I added a jQuery set to the attribute tests and found that, in > addition to all manner of inconsistencies and errors in IE, FF throws > an exception on using attr to set DOM0 event handler properties (due > to the aforementioned Function to string conversion). Why would you do this, other than to break things? Obviously the code is still not perfect, but as long as you use it for its intended purposes, does it cause problems? I don't think a lib should be bullet-proof against users trying to do things that they aren't supposed to do. Matt Kruse
From: Hans-Georg Michna on 7 Dec 2009 18:03 On Mon, 7 Dec 2009 14:33:43 -0800 (PST), Matt Kruse wrote: >On Dec 7, 3:59�pm, David Mark <dmark.cins...(a)gmail.com> wrote: >> removeAttr(el, 'colspan'); // Boom >Why would you do this, other than to break things? Because some user wants to replace a colspanned table cell with two not colspanned ones? That's not far-fetched at all. I'd say, send John Resig some qUnit tests. (:-) Hans-Georg
From: "Michael Haufe ("TNO")" on 7 Dec 2009 18:04
On Dec 7, 4:33 pm, Matt Kruse <m...(a)thekrusefamily.com> wrote: > Why would you do this, other than to break things? Quality code is measured not only by common usages, but by how it handles edge cases as well. I for one welcome these types of reviews, not having time to do them myself. > Obviously the code is still not perfect, but as long as you use it for > its intended purposes, does it cause problems? If a method claims to remove an attribute, you would assume it could remove an attribute regardless of what it is. > I don't think a lib should be bullet-proof against users trying to do > things that they aren't supposed to do. If you're a developer trying to maintain someone else's code, and you see that they are using library X, it would be nice to know that you can look at a method library and assume it does what it's advertised to do. |