Poorly written jQuery
jcready - January 13, 2009 - 17:55
| Project: | SimpleMenu |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | postponed (maintainer needs more info) |
Description
I don't even know how your code works for most people. It shouldn't work. For instance, you have the giant JS var called simplemenu at the bottom containing the HTML for the menu. Then at the very top of simplemenu.js you have this:
var menu = $(simplemenu).attr("id", "simplemenu").addClass('clear-block');
You can't do this, it make absolutely no sense. jQuery takes a selector for the parameter, you can't just pass it a bunch of HTML as a string. Attached is a correctly written simplemenu.js.
| Attachment | Size |
|---|---|
| simplemenuFIXED.js_.txt | 1.23 KB |
#1
Edit: Changed "$(element).appendTo(simplemenu);" to "$(element).append(simplemenu);"
#2
jQuery can take more than just a selector as a parameter. Among other things it can take a string of html, which in turn creates DOM elements from the html string which can then be inserted into the document or manipulated just like any other elements.
#3
@Roger López: You are correct sir, I retract my previous issue. But now I have another.
At the top of simplemenu.js you have:
var menu = $(simplemenu).attr("id", "simplemenu").addClass('clear-block');Then later in the code you have:
$(menu).superfish( { blah, blah, blah... } );So you're passing a jQuery object into another jQuery object...
Shouldn't the code be just
menu.superfish( { blah, blah, blah... } );sincemenuis already a jQuery object.#4
true. jQuery is smart enough to realize that the argument is already a jQuery object, but for correctness and performance, you are correct.
#5
Actually, it looks like menu is *not* an object all the time. When theme exclusion is used, simplemenu does not exist in those themes... Internet Explorer and Firefox are throwing errors and the error possibly breaking stuff in IE, but I guess this is a seperate issue.