Search and Top Navigation
#5557 closed bug (fixed)
Opened April 27, 2010 06:22PM UTC
Closed April 28, 2010 02:43AM UTC
Last modified May 20, 2010 11:14AM UTC
Comparing element.nodeName directly against upper case tag names can cause problems
Reported by: | complex | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.8.2 |
Component: | ui.accordion | Version: | 1.8 |
Keywords: | nodeName comparison case sensitivity sensitive lower upper ul | Cc: | |
Blocked by: | Blocking: |
Description
Environment
Firefox 3.0.19
jQuery 1.4.2
jQueryUI 1.8
Issue
Just found this in jquery.ui.accordion.js (accidentally):
if (this.element[0].nodeName == "UL")
According to my past experience it is not good this way, since nodeName can be lower case sometimes, depending on the type of the document and maybe on the browser, I'm not sure. It is error prone this way, I'd suggest:
if (this.element[0].nodeName && this.element[0].nodeName.toLowerCase() == "ul")
Can you come up with a better solution?
(I've just found many instances of similar direct nodeName comparison issues in the DataTables plugin a few minutes ago and searched my code base as a whole for similar issues. I found the above line in jQuery UI this way. So I can't reproduce it, but I observed that the nodeName of a <td> element is "td" on my browser instead of "TD", so it should be the same for "UL" as well.)
Attachments (0)
Change History (3)
Changed April 27, 2010 06:28PM UTC by comment:1
Changed April 28, 2010 02:43AM UTC by comment:2
milestone: | TBD → 1.9 |
---|---|
resolution: | → fixed |
status: | new → closed |
Fixed in 0aa4c7f by just removing the check. Since we're already filtering the children to li's, we don't need to check the type. But to answer your question, the best way to do this is to use .is( "ul" ).
Changed May 20, 2010 11:14AM UTC by comment:3
milestone: | 1.9 → 1.8.2 |
---|
Additional information
This problem seems to happen only when the element in question is generated by JavaScript code. It happened in the case of my DataTable instance and applying the above
toLowerCase()
fix solved that issue.