Fix for DomQuery's getElementById 'bug'

[simpsora]

Hi,

I've been using DomQuery with great success lately -- it's an excellent library.

Recently, I modified my copy of DomQuery to work around an IE annoyance (some would call it a bug), and I wanted to submit it back to you, in case you wanted to include it in the source.

The issue is with getElementById(). In IE, if an element with the given id value is not found, it will fall back to searching for an element whose name attribute matches the given id value, and return the first match.

The fix merely compares the passed-in id against the id of the returned element, and either returns the element if they match, or null if not.
Since DomQuery calls getElementById() three times, I created a convenience method to do the check. The diff (against http://www.yui-ext.com/deploy/ext-1.0-alpha3/source/core/DomQuery.js) follows:


$ diff -u DomQuery.js DomQuery_rs.js
--- DomQuery.js 2007-04-06 11:39:24.000000000 -0600
+++ DomQuery_rs.js 2007-04-06 11:48:11.000000000 -0600
@@ -284,11 +284,16 @@
}
return r;
}
+
+ function gebi(parent, id) {
+ var r = parent.getElementById(id);
+ return r && r.id === id ? r : null;
+ }

function quickId(ns, mode, root, id){
if(ns == root){
var d = root.ownerDocument || root;
- return d.getElementById(id);
+ return gebi(d, id);
}
ns = getNodes(ns, mode, "*");
return byId(ns, null, id);
@@ -381,7 +386,7 @@
root = document;
}
if(typeof root == "string"){
- root = document.getElementById(root);
+ root = gebi(document, root);
}
var paths = path.split(",");
var results = [];
@@ -448,7 +453,7 @@
*/
is : function(el, ss){
if(typeof el == "string"){
- el = document.getElementById(el);
+ el = gebi(document, el);
}
var isArray = (el instanceof Array);
var result = Ext.DomQuery.filter(isArray ? el : [el], ss);

Thanks

[jack.slocum]

I initially started to patch this in, but then I thought about it and ALOT of Ext could break if you have a mix of names/ids that are the same.

IMO a better solution is to promote awareness of this issue and work around it by using names that won't clash with IDs. Providing a workaround in only 1 class seems like a poor solution.

What do you think?

[simpsora]

I initially started to patch this in, but then I thought about it and ALOT of Ext could break if you have a mix of names/ids that are the same.

IMO a better solution is to promote awareness of this issue and work around it by using names that won't clash with IDs. Providing a workaround in only 1 class seems like a poor solution.

What do you think?

I have mixed feelings about it.
In the interests of readable (and maintainable) code, and semantic markup, there are reasonable cases where you might have different elements, one having the id of another's name.

Really, the only issue with doing so how IE (mis?)handles it.

You are right though; it doesn't make sense to only fix it in one spot. Ideally it would be fixed everywhere, but as I don't know much about the rest of Ext, I have no idea what that would entail.

For now, I'll keep using my modified copy, and keep an eye on what develops.

Thanks again.