From: "Michael Haufe ("TNO")" on 7 Dec 2009 18:07 On Dec 7, 5:04 pm, "Michael Haufe (\"TNO\")" <t...(a)thenewobjective.com> wrote: > 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. correction: "method library" -> "method of that library"
From: David Mark on 7 Dec 2009 18:20 On Dec 7, 5:33 pm, Matt Kruse <m...(a)thekrusefamily.com> 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? Why would I do what? Remove an attribute? To get back to the default colspan, I expect. Why does the method exist at all? And you do understand that this is but one example. > > > 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? See above. > > Obviously the code is still not perfect, but as long as you use it for > its intended purposes, does it cause problems? First you would have to define its intended purposes. The logic of jQuery's attr/removeAttr "pair" demonstrates a stunning lack of comprehension about basic DOM operations? The unfounded assumptions that stick out are: 1. Attributes and properties are the same thing 2. All versions and of IE treat attributes and properties the same As for #1, the domain and range for an attribute-based function is quite different from that of a property-based function. If you look at what jQuery does in attr, it clearly shows a botched design that has been fiddled with just enough to make their limited unit tests work. There's no rational logic present, so clearly the authors have no idea what they are doing with attributes or properties. (!) As for #2, pressing the compatility mode button in IE8 changes the behavior of these functions. As these functions (attr at least) are used in every jQuery example and book ever written, it seems ludicrous that they should fail even in an IE8-only environment (unless you somehow locked out compatibility mode as an option). And the mere demonstration (even if these functions weren't used pervasively) of such monumental incompetence and apathy (as you know, they've been told about this problem numerous times) should be enough to convince you that the effort is worthless. If a DOM library makes it _harder_ to read/write/remove attributes and/or read/write properties (note the difference), what purpose is it serving? Browsers don't have any trouble doing the latter and you rarely need to do the former. It also displays that the unit testing is worthless. How can it pass such obviously broken logic (and why would you need them to tell you it is wrong?) When you write software as a series of haphazard observations, you end up with a haphazard collection of unit tests, each confirming a sighting. It's the bugs (or implementation variations) they don't see (and therefore don't test for) that cause the problems. > > I don't think a lib should be bullet-proof against users trying to do > things that they aren't supposed to do. What the hell does that mean? What is it you think these functions are supposed to do? Is there some white list of attributes/properties that are allowed by jQuery? And I can't believe I'm asking you any of this. Are you some other Matt Kruse or are seriously starting this ridiculous discussion again? jQuery does _not_ simplify DOM scripting. The authors do _not_ understand DOM scripting or the Javascript language and especially not IE. The CSS selector stuff is ludicrous and incompatible with QSA. Almost every browser "supported" by jQuery _has_ QSA now anyway. The animations are third-rate, everything it does is incredibly slow, inefficient and resource intensive. It leaks memory, throws exceptions, sets expandos and many other bad practices (and yes it still uses forms of browser sniffing too). And the much-cited documentation is horrible (look up attr and removeAttr for examples). You couldn't pick a worse script and you know it (and the whole idea of picking one script for every context is the ludicrous anyway). I think that about says it. Feel free to silently skulk off.
From: RobG on 7 Dec 2009 20:10 On Dec 8, 8:33 am, Matt Kruse <m...(a)thekrusefamily.com> 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? To test the code? Given the vaguaries of browsers, I would expect a unit test for library methods that add and remove HTML attributes and properties would test every standard attribute and property in as many browsers as is reasonable - adding, modifying and deleting, including cases where attributes are set or omitted in the source HTML. [...] > Obviously the code is still not perfect, but as long as you use it for > its intended purposes, does it cause problems? If there are shortcomings, they should be documented. Paricularly if there are methods to modify attributes and properties that will fail with particular values that should be expected to work. > I don't think a lib should be bullet-proof against users trying to do > things that they aren't supposed to do. You can only say users "aren't suppposed to do" something if either it is *very* well known that something causes an issue or there's documentation to say "don't to it". Is there any jQuery documentation listing the standard HTML attributes that won't be correctly handled by removeAttr? -- Rob
From: rf on 7 Dec 2009 20:16 RobG wrote: > On Dec 8, 8:33 am, Matt Kruse <m...(a)thekrusefamily.com> 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? > > To test the code? > > > Is there any jQuery documentation listing the standard HTML attributes > that won't be correctly handled by removeAttr? Perhaps the function should be called removeSomeAttrsButPotLuckWhichOnes.
From: Matt Kruse on 8 Dec 2009 16:07
On Dec 7, 5:48 pm, David Mark <dmark.cins...(a)gmail.com> wrote: > On Dec 7, 6:04 pm, "Michael Haufe (\"TNO\")" > > 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. Agreed. I make exceptions for client-side js, because the size and speed of code is important. It is better to make it clear in the API docs what should and shouldn't be passed into functions rather than doing lots of type-checking of arguments, etc. That stuff can be left in a "debug" version, however. As David says early in his criticism: > // don't set attributes on text and comment nodes > Don't pass them! Put that in the docs. :) .... > > I for one welcome these types of reviews, > > not having time to do them myself. > Seems most who don't are using or advocating the library in question. I definitely welcome these kinds of reviews and criticisms. I only wish there was less snarkiness, better formatting, and a lot less bias. I am thankful for David's continued criticism of jQuery et al, because it lets people know of the short-comings and also points of things that the developers can work on. These aren't perfect solutions, but are works in progress. jQuery especially has made progress and addressed some criticisms like browser sniffing, etc. Yet some people ignore the progress and instead focus on the stuff that still needs work. > > > 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. That may not be a justified assumption. It can do whatever it wants, but should be clear and documented. There are exceptions and unsupported conditions to many methods and algorithms. If the docs are not clear about the exceptions and situations where the assumed behavior will not work as you might expect, then the docs should certainly be corrected if the code is not. For example, obviously the attr() method is meant to only set string properties. I'm not sure why anyone would want to pass a function(){} to attr() when you can use the event methods instead. But as David points out, if you try to it will fail. This kind of stuff should be clearly documented. I don't think it's a bug (it's unintended use of the function) but the decision to NOT work in this way should be intentional, not just an unintended side-effect of poor coding. Which may be the case here, I don't know. Matt Kruse |