Re: Javascript Library
- From: Matt Kruse <matt@xxxxxxxxxxxxx>
- Date: Thu, 01 Nov 2007 15:41:45 -0000
On Nov 1, 1:06 am, David Mark <dmark.cins...@xxxxxxxxx> wrote:
Google as an example?! The worst JavaScript developers in history?
Same goes for Amazon.
I don't think you can dismiss the "big guys" so easily. These
companies are worth billions of dollars and they are doing it by using
a strategy that you say is terrible and will never work. Clearly, it
does work. When you can showcase the kind of success that these
companies have by using YOUR methodology, then maybe your criticisms
will have more weight.
And the
question remains: what sort of XML document runs script in IE?
XML can be returned by ajax calls and operated on.
Oddly enough, I don't have any trouble setting the type attribute for
input elements, dynamically created or not.
I'd be curious to see your code, as I've never gotten it to work in
IE. Always had to resort to creating a new input of the new type and
removing the old.
I bet you would. My test suite covers quite a few more agents than
yours.
Code you post your test suite, or give a url? If you have test cases
that break other libraries or functions, it would be great to make
them available. And you wouldn't even need to share your precious
code.
You read it backwards apparently. The point is that the problem may
exist in browsers that are NOT IE. Why make stupid assumptions? Here
is that portion of the code:
if ( jQuery.browser.msie ) {
for ( var i = 0; second[i]; i++ )
if ( second[i].nodeType != 8 )
first.push(second[i]);} else
for ( var i = 0; second[i]; i++ )
first.push(second[i]);
What's wrong with that picture?
Wow, I hadn't looked at that portion of the code yet. Things like that
certainly concern me.
The difference between you and me is that I don't give away code for
beer money donations.
You should. Free beer is fantastic.
I don't have to assume. I have dealt with all of the same problems
that you are stumbling over.
And solved them, presumably? And then locked those awesome solutions
up in the closet so no one else can benefit from them?
I think publicly-available code with some problems is more valuable
than robust, "perfect" code that is locked away. The public code will
be inspected, torn apart, improved, and re-thought on a regular basis.
It will evolve into the best solution. The code kept private will rot
and not benefit from public criticism and improvements. So, while your
code may be "better" than that in jQuery, it is of no use to anyone
but yourself, so I'm not sure why you keep mentioning it.
Does it indicate Mozilla? IE has spoofed Mozilla in its userAgent
string since the beginning. So is IE Mozilla?
jQuery correctly identifies IE as IE, not Mozilla.
GC is automatic. Do you understand that a memory leak is an exception
to GC? Your comment indicated that you thought the event might leak.
What do you base that suspicion on? Do you automatically remove every
listener every time the page unloads, just in case it might cause a
memory leak?
Memory leaks are a complex issue, especially in IE. I'm not sure if
you're aware of every possible leak scenario, but I know I'm not.
There are ridiculous leak scenarios that don't even involve closures
or circular references or events. Removing event listeners in this
case may not be required to avoid leaks, but it's perhaps a safe,
conservative practice.
if ( typeof a != "array" )
What (in any JavaScript implementation) will evaulate that to false?
What is an array in JavaScript?
I hope someone somewhere is blushing.
You are blind. Others certainly did. Even the jQuery fanboy Matt
Kruse agreed with enough of it to open up tickets on your site.
I didn't actually open tickets, I just started a discussion.
FYI, some of your criticisms are certainly valid, and I posted your
entire original post to the jQuery dev list. I also followed up with
my own post below that goes into detail on the attr() function. You
are welcome to critique my critiques. I already see a few places that
my suggestions could be improved.
=== Begin Copied Post From jQuery-Dev ===
On Oct 30, 10:18 pm, Matt <m...@xxxxxxxxxxxxx> wrote:
I haven't had time yet to look into the code in detail, but I will do
so in the next few weeks.
I took a look at the 'attr' function today and here are my comments.
I'm not going to open a ticket for specific items, but I'd like to
hear some feedback. If these types of comments are helpful I will
continue looking at the code and making suggestions. If I am way off
base, then I'll just be quiet ;)
I've copied the entire attr() function from the nightly build below
and added comments:
attr: function( elem, name, value ) {
var fix = jQuery.isXMLDoc( elem ) ?
{} :
jQuery.props;
// Safari mis-reports the default selected property of a hidden option
// Accessing the parent's selectedIndex property fixes it
if ( name == "selected" && jQuery.browser.safari )
elem.parentNode.selectedIndex;
Why even do the name and browser check? Surely no browser will choke
on simply accessing the selectedIndex property. Just do it blindly.
// Certain attributes only work when accessed via the old DOM 0 way
if ( fix[ name ] ) {
if ( value != undefined )
elem[ fix[ name ] ] = value;
return elem[ fix[ name ] ];
} else if ( jQuery.browser.msie && name == "style" )
return jQuery.attr( elem.style, "cssText", value );
else if ( value == undefined && jQuery.browser.msie && jQuery.nodeName( elem, "form" ) && (name == "action" || name == "method") )
return elem.getAttributeNode( name ).nodeValue;
If you're going to include this fix for 'action' and 'method' then why
not also for 'accept','enctype','name','target', etc?
And why restrict it only to IE? In fact, why even use getAttribute()?
Why not just use elem.getAttributeNode( name ).nodeValue for all node
values in all browsers?
(I've not researched that one, so there is a potential that fails in
some browsers)
You could also just do a general fix in the code below... (Marked #1)
// IE elem.getAttribute passes even for style
else if ( elem.tagName ) {
if ( value != undefined ) {
// We can't allow the type property to be changed (since it causes problems in IE)
if ( name == "type" && jQuery.nodeName( elem, "input" ) && elem.parentNode )
throw "type property can't be changed";
Do other browsers allow it? Why fail for all browsers, if only IE has
a problem?
elem.setAttribute( name, value );
Now, what if I am setting the 'action' attribute of a form that has an
input with name 'action'? It will safely get to this block in IE and
fail to set the attribute. You've handled this case for getting the
value, but not setting. Calling form.attr('action','val') in this case
will not set the action attribute in IE.
}
if ( jQuery.browser.msie && /href|src/.test( name ) && !jQuery.isXMLDoc( elem ) )
return elem.getAttribute( name, 2 );
What harm would there be in always passing a '2' to this call in all
browsers? Browsers other than IE would ignore it, and IE would always
use it to return the actual node value. Why not get rid of the browser
check and condition and just call it blindly?
In this specific case, I see that IE has a problem with passing '2' in
the case above where we are trying to get the 'action' attribute. But
the fix below avoids that. The general point is, why only limit things
like this to a browser-sniffed IE, when it wouldn't hurt to apply the
same fix for every browser?
return elem.getAttribute( name );
Here you could put in a general fix (#1) that would do away with the
special case handling above:
var v = elem.getAttribute(name);
return ((v && v.tagName)?
elem.getAttributeNode(name).nodeValue:elem.getAttribute(name,2));
This seems to cover all cases, doesn't it? If future cases are found
where certain attributes are 'masked' by returning elements, the case
will automatically be handled. And it's not limited to any specific
browser.
I haven't tested anything other than IE, but I would be interested to
hear if it causes any problems.
// elem is actually elem.style ... set the style
I don't understand why opacity, filter, etc are even considered
attributes.
} else {
// IE actually uses filters for opacity
if ( name == "opacity" && jQuery.browser.msie ) {
Will it ever even get here if it's not IE?
if ( value != undefined ) {
// IE has trouble with opacity if it does not have layout
// Force it by setting the zoom level
elem.zoom = 1;
// Set the alpha filter to set the opacity
elem.filter = (elem.filter || "").replace( /alpha\([^)]*\)/, "" ) +
(parseFloat( value ).toString() == "NaN" ? "" : "alpha(opacity=" + value * 100 + ")");
}
return elem.filter ?
(parseFloat( elem.filter.match(/opacity=([^)]*)/)[1] ) / 100).toString() :
"";
}
name = name.replace(/-([a-z])/ig, function(all, letter){
return letter.toUpperCase();
});
if ( value != undefined )
elem[ name ] = value;
return elem[ name ];
}
}
I hope these comments make sense!
Matt Kruse
.
- Follow-Ups:
- Re: Javascript Library
- From: David Mark
- Re: Javascript Library
- References:
- Re: Javascript Library
- From: John Resig
- Re: Javascript Library
- From: David Mark
- Re: Javascript Library
- Prev by Date: Re: Form Submission to New Window problem
- Next by Date: Unicode + Escape Character = IE Unterminated string constant error!
- Previous by thread: Re: Javascript Library
- Next by thread: Re: Javascript Library
- Index(es):