Closed
Bug 863021
Opened 11 years ago
Closed 7 years ago
getElementsByClassName failing in firefox 20 when prototype-deprecation script is used
Categories
(Web Compatibility :: Site Reports, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: schenn, Unassigned)
Details
Attachments
(1 file)
113.03 KB,
application/zip
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 Build ID: 20130409194949 Steps to reproduce: Using jQuery 1.8.3, firefox 20.0.1 var entity = jQuery('div').find('.class'); Actual results: The following error was thrown: "TypeError: can't convert undefined to object" inside the Sizzle function at line 3930. Changing: "push.apply( results, slice.call(context.getElementsByClassName( m ), 0) );" to : "push.apply( results, slice.call(context.querySelectorAll( '.' + m ), 0) );" solves the issue, however updating jQuery or using google's CDN negates this solution. As you can see, the issue most likely lies with getElementsByClassName Expected results: The corresponding element with the given class name or an empty jQuery object should have been returned instead of an error. I am not the only person to have this issue: https://support.mozilla.org/en-US/questions/955992 http://stackoverflow.com/questions/15946734/jquery-not-working-on-firefox-20/16069781#16069781
Please attach (or provide a link to) a small testcase that shows the problem.
Flags: needinfo?(schenn)
Keywords: testcase-wanted
Reporter | ||
Comment 2•11 years ago
|
||
Working on it. Taking the problem page and removing the CMS fluff and stuff to provide a good test case.
Flags: needinfo?(schenn)
Reporter | ||
Comment 3•11 years ago
|
||
Test Case attached. Yea these are outdated libraries, you try telling the Magento CMS that. Removing the prototype-deprecation-1.6.0.2.js library stops the error from firing as well, however that's embedded in our CMS and cant be changed at the moment. The issue only exists in FF 20.X, it does not fire in FF < 20, IE 7,8,9, Opera 12, or Chrome. By stepping through the sizzle function at the point of failure, it appears as though context.getElementsByClassName(m) when the context is div#test is returning undefined instead of an html object.
Updated•11 years ago
|
Attachment #739346 -
Attachment mime type: application/octet-stream → application/zip
The deprecation.js script successfully changes getElementsByClassName in Firefox 20 from native code to a notification function. In earlier versions of Firefox, getElementsByClassName remains native code. Could this be related to one of the Element/HTMLElement changes?
Comment 5•11 years ago
|
||
Very much so. Before Firefox 20, each DOM element prototype had its own copy of getElementsByClassName. Now for objects on WebIDL bindings there is only one copy, as per spec, on Element.prototype. But that's also the case in other browsers, so that on its own shouldn't be a problem. Might be interacting with something else.... In any case, I can reproduce on the testcase in comment 3. Looking into this now.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 6•11 years ago
|
||
Oh. I know why it works in other browsers. It's because this prototype-deprection thing conditions all its dirty work with: if (!Prototype.Browser.Gecko) return; so it only screws up the method in Gecko.
Comment 7•11 years ago
|
||
Alright, so here's the story: 1) This version of prototype only defines Element.Methods.getElementsByClassName if !document.getElementsByClassName, which is clearly false in Gecko. 2) The prototype-deprecation script tries to deprecate the nonexistent Element.Methods.getElementsByClassName. But only in Gecko-based browsers. 3) The deprecation logic, once you unroll it, looks somewhat like this: var obj = Element.Methods.getElementsByClassName = ( Element.Methods.getElementsByClassName || function() {}).wrap(something); What wrap() does is create a new function which will invoke the "something" passed in, passing it as an argument the "this" value that wrap() was invoked on. And in this case the "something" does: function(proceed) { /* some irrelevant stuff */ return proceed.apply(proceed, args); } The upshot of all that is basically as if the script had done: Element.Methods.getElementsByClassName = function() {}; and then of course it calls Element.addMethods, which copies stuff from Element.Methods to Element.prototype, and now your getElementsByClassName on an element always returns undefined. As far as I can tell this is just a bad bug in prototype-deprecation that was being hidden but now bits us... but only us, since again they only run all this junk if "Gecko" and not "KHTML".
Comment 8•11 years ago
|
||
I'm not sure there's anything we can do on our end here short of adding "KHTML" to the UA string or totally reverting the fix to only put things on Element.prototype. Given that both are non-options, I guess we just have to convince people to stop using this broken script. :(
Keywords: testcase-wanted
Summary: getElementsByClassName failing in firefox 20 → getElementsByClassName failing in firefox 20 when prototype-deprecation script is used
Comment 9•11 years ago
|
||
Reporter, do you know how I could get in touch with this CMS vendor that's shipping this?
Flags: needinfo?(schenn)
Reporter | ||
Comment 10•11 years ago
|
||
It looks like they cut the file out in the current version which is newer then the one we have. We're using the magento CMS v 1.5, and the current working is 1.7 and it's not there. Migrating between versions is not easy due to their complete lack of quality documentation (generated docs, wiki has many many broken links), but that's what they pay us for. Thank you so much for your help in figuring out this issue. For what its worth, their website is http://www.magentocommerce.com/ I'll be sure to post on their forum in the next day or so about the issue to inform the other users of the issue and solution.
Flags: needinfo?(schenn)
Comment 11•11 years ago
|
||
For what it's worth, if you're in a position to edit the prototype-deprecation file, just removing these lines: { methodName: 'getElementsByClassName', namespace: Element.Methods, message: 'Element#getElementsByClassName has been deprecated, please use Element#select instead.' }, should do the trick. Or even replacing "!Prototype.Browser.Gecko" with "true" in the early returns. ;) And thank _you_ both for filing this and for putting together a testcase that made it easy to tell what was going on!
Reporter | ||
Comment 12•11 years ago
|
||
And thank you for pointing out that block so that I have a specific fix to share with the other users.
Comment 13•7 years ago
|
||
Over to tech evanglism, though it's not obvious whether this is still being an issue...
Component: DOM → Desktop
Product: Core → Tech Evangelism
Version: 20 Branch → Firefox 20
Comment 14•7 years ago
|
||
I think we can probably just close this at this point. The relevant code in prototype was removed a few years back (or some evolved version of it): https://github.com/sstephenson/prototype/commit/54f3695a99f31eb65acd45863a36c1a10ba40188#diff-bcc9075cd345ac004634f523f353e59a We can deal with sites on a one-off basis if they're using an older version of Magento. (Closing as WORKSFORME, because it seems better than INVALID or FIXED in this case)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Updated•5 years ago
|
Product: Tech Evangelism → Web Compatibility
You need to log in
before you can comment on or make changes to this bug.
Description
•