User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.142 Safari/535.19 Steps to reproduce: I tried this code: Element.prototype.test = 'test'; var svg = document.getElementsByTagName('svg')[0]; console.assert(svg.test == 'test'); Actual results: The assertion failed Expected results: the assertion shouldn't have failed.

This bug breaks MooTools Element methods on <svg> elements: https://github.com/mootools/mootools-core/issues/2331

Hey guys, any reason why SVG elements are failing to pick up their prototypes as Arian indicates? If this is not expected behavior, it could affect the assumptions and element-centric code of many developers, please advise.

Looks like SVGElement.prototype has Object.prototype as its proto instead of Element.prototype. I'll look into why on Monday if no one else has before then. If this is a regression, someone finding the regression range would be a big help.

Maybe interesting to note is that: Element.prototype.test = 'yo' document.createElementNS('http://www.w3.org/2000/svg', 'svg').test // → undefined in FF, but 'yo' in chrome document.createElement('svg').test // → 'yo' in both FF and Chrome

(In reply to Arian Stolwijk from comment #4) > Maybe interesting to note is that: > > Element.prototype.test = 'yo' > document.createElementNS('http://www.w3.org/2000/svg', 'svg').test > // → undefined in FF, but 'yo' in chrome > document.createElement('svg').test > // → 'yo' in both FF and Chrome The latter creates an HTMLElement.

So the issue is that DOM_CLASSINFO_MAP_BEGIN_NO_CLASS_IF(SVGUnknownElement, nsIDOMSVGElement) means that when we're setting up the proto for SVGElement we end up using the interfaceinfo for nsIDOMSVGElement for the classname. So we recursively look up "SVGElement" on the window, look for .prototype on it, get nothing (since we're still setting it up) and end up using Object.prototype as the proto. Peter, is the NO_CLASS_IF part here correct? Why do we not want to use the parent in the NO_CLASS_IF case, exactly?

Probably a regression from bug 82235.

Probably a regression from bug 589640.

The NO_CLASS_IF bit was added by me as a test bustage fix for bug 589640, but I didn't write down what was broken, sadly. I tried pushing a backout of the NO_CLASS_IF bit to try, and it seems to pass tests... It also fixes this bug as far as I can tell.

Comment on attachment 613305 [details] [diff] [review] Make SVGElement.prototype be Element.prototype again. Requesting approval for m-c. I think this should be a pretty safe fix, and it's a basic web compat issue that we should fix.

Just for posterity, the reason removing NO_CLASS_IF is ok is that SVGUnknownElement is the DOMCI for SVGElement, so the primary interface is the class interface (nsIDOMSVGElement).

Comment on attachment 613305 [details] [diff] [review] Make SVGElement.prototype be Element.prototype again. [Triage Comment] Approved for mozilla-central - this is a web compatibility bug, and we don't expect it to have a major effect on any of the main FN use cases.

It seems to me that we should put this regression fix on aurora/beta as well.

Comment on attachment 613305 [details] [diff] [review] Make SVGElement.prototype be Element.prototype again. Yeah, that's not a bad idea. I hadn't wanted to rush it into 12 before. [Approval Request Comment] Regression caused by (bug #): 589640 User impact if declined: Some sites' JS doesn't work right. Testing completed (on m-c, etc.): Checked in on m-c, passing tests. Risk to taking this patch (and alternatives if risky): Slight risk that something could go wrong, but I'm not really sure what it would be. Never bet against some website _somewhere_ breaking, though. ;) String changes made by this patch:

Comment on attachment 613305 [details] [diff] [review] Make SVGElement.prototype be Element.prototype again. This made the cutover.

[Triage Comment] Approved for beta and updating the status flags to show it's not yet on beta (but made the cut to aurora) - please update flag once this has landed.

Can we move ahead with landing the approved patch on FF13?

Er, yes. I didn't set the flag, so didn't get the notification mail....

Hm. I _did_ set the flag. I wonder what happened to that mail. In any case, http://hg.mozilla.org/releases/mozilla-beta/rev/05cfe44826dd

The automated test for this bug passed on all OSs on Firefox 13.0 beta: https://tbpl.mozilla.org/php/getParsedLog.php?id=12163160&full=1&branch=mozilla-beta https://tbpl.mozilla.org/php/getParsedLog.php?id=12163930&full=1&branch=mozilla-beta https://tbpl.mozilla.org/php/getParsedLog.php?id=12159488&full=1&branch=mozilla-beta I have also tried to verify this fix manually and the assertion from comment 0 doesn't fail on Firefox 13.0b6. The problem is that it also doesn't fail on Firefox 12.0 on my workstation. Could someone please let me know how to reproduce this issue? Until now I have tried loading the Element.svg.html attachment in Firefox 12.0 and looking in the Error and Web consoles for the assertion failure. Also tried with replacing the corresponding lines from that attachment with Element.prototype.test = 'test'; var svg = document.getElementsByTagName('svg')[0]; console.assert(svg.test == 'test'); and looking in the Error and Web consoles for the assertion failures.

> The problem is that it also doesn't fail on Firefox 12.0 on my workstation. That's because there is no console.assert method. > Could someone please let me know how to reproduce this issue? Replace console.assert by alert() and see what boolean gets alerted.

Boris, thank you for you suggestion. I didn't get any error about using an inexistent method, so I didn't know for sure what the problem was. Verified as fixed with the guidelines from comment 0 and comment 24: Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0 BuildID: 20120528154913

The automated test for this bug passed on all OSs on Firefox 14.0 beta too: https://tbpl.mozilla.org/php/getParsedLog.php?id=12407667&full=1&branch=mozilla-beta https://tbpl.mozilla.org/php/getParsedLog.php?id=12407415&full=1&branch=mozilla-beta https://tbpl.mozilla.org/php/getParsedLog.php?id=12410453&full=1&branch=mozilla-beta Also verified as fixed with the guidelines from comments 0 and 24 on: Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0 (20120605113340)

No point for developer docs for this. It might have been useful in the Fx14 docs, but that ship has long sailed.