817420 - new bindings api for element broke cloud9/apf

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Axel Hecht [:Pike]]

I don't really know why, but bug 814195 broke cloud9, aka, c9.io.

I get a js error:

TypeError: this.data.selectSingleNode is not a function

Works for me in 2a76899feb06, but not in c1425a30de0e, in a local debug build.

I don't know why this changes, but I'm concerned about the web-compat impact of that change.

I'll ask Ruben Daniels, who apparently wrote most of apf to figure out what that code is actually doing.

Comment 1

[Axel Hecht [:Pike]]

Ruben came up with a test case in email, I checked that 

Element.prototype.test0987 = function(){return "mocked"};
var xmlParser = new DOMParser();
xmlParser = xmlParser.parseFromString("<test />", "text/xml");
xmlParser.documentElement.test0987();

returns "mocked" on 17, and 

/*
Exception: xmlParser.documentElement.test0987 is not a function
@Scratchpad/1:4
*/

on nightly

Comment 2

[Boris Zbarsky [:bzbarsky]]

Presumably proto chain weirdness for things whose proto should be Element.prototype; there's a good chance they're picking up the "normal" xpconnect prototype instead of the right thing (while other things properly chain up to Element.prototype because we get it when we look up HTMLElement.prototype's proto, say).

Comment 3

[Boris Zbarsky [:bzbarsky]]

And I'm a little surprised we have no test for that....  :(

Comment 4

[Boris Zbarsky [:bzbarsky]]

Quick test in web console:

--
[18:25:38.905] HTMLElement.prototype.__proto__ == Element.prototype
[18:25:38.906] true
--
[18:25:57.072] document.createElementNS(null, "foo").__proto__ == Element.prototype
[18:25:57.074] false
--
[18:30:04.958] document.createElementNS(null, "foo").__proto__
[18:30:04.960] [object XPC_WN_ModsAllowed_NoCall_Proto_JSClass]

Comment 5

[Boris Zbarsky [:bzbarsky]]

Axel, any way I can test the original site the problem came up on to make sure that a fix really fixes that site?

Comment 6

[Axel Hecht [:Pike]]

https://c9.io/, you can just log in with github or bitbucket credentials, or sign up.

Comment 7

[Axel Hecht [:Pike]]

to clarify, c9 isn't broken in a subtle way. The IDE doesn't load at all really. You end up with a blank screen instead of your repos and editor pane. Once you can open a file, you should be well past the regression.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Some hackery that doesn't quite work

This asserts in nsDOMClassInfo::PostCreatePrototype.  Not sure whether we can skip it for the case when we're using a WebIDL proto...  Further, this doesn't do the right thing in terms of telling the XPCWrappedNativePrototype that the JS object has been finalized.  :(  Maybe it would be better to just have a fast method of getting the right proto from the nsXPCClassInfo and just use that on every XPCWrappedNative/SlimWrapper creation instead of using the XPCWrappedNativeProto's JSObject?  On the other hand, maybe the right solution here is to switch over basic Element instances, and maybe back out the quickstubs for Element until that happens?  :(

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: Quick fix v1

How about this as a quick fix for now?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 687680 [details] [diff] [review]
Quick fix v1

Hmm.  So this all still works because we keep putting the WebIDL "quickstubs" on the XPConnect Element.prototype as needed, right?

Good idea and r=me, though you should run tests on this; some of the test_interfaces stuff is likely to fail again with this change and we'll have to readd some of the json known-fail goop.

Comment 11

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2a7197560225

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2a7197560225

Comment 13

[Axel Hecht [:Pike]]

VERIFIED on nightly, thanks.