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)

Firefox 20
x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: schenn, Unassigned)

Details

Attachments

(1 file)

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
Component: Untriaged → DOM
Product: Firefox → Core
Please attach (or provide a link to) a small testcase that shows the problem.
Flags: needinfo?(schenn)
Keywords: testcase-wanted
Working on it. Taking the problem page and removing the CMS fluff and stuff to provide a good test case.
Flags: needinfo?(schenn)
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.
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?
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
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.
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".
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
Reporter, do you know how I could get in touch with this CMS vendor that's shipping this?
Flags: needinfo?(schenn)
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)
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!
And thank you for pointing out that block so that I have a specific fix to share with the other users.
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
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
Product: Tech Evangelism → Web Compatibility
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: