Can we make nsINode::WrapObject final now?

No (see for example Element::WrapObject).

Comment on attachment 824031 [details] [diff] [review] v1 >+nsWindowSH::PostCreate(nsIXPConnectWrappedNative *wrapper, >+ NS_ASSERTION(!sgo || sgo->GetGlobalJSObject() == nullptr, How could sgo be null here? We should assert it's not, I would think. >- static inline bool IsReadonlyReplaceable(jsid id) Now that IsReadonlyReplaceable is gone, can you get rid of some of those jsids it was using? >+++ b/dom/imptests/failures/html/html/browsers/the-window-object/test_window-properties.html.json Followup to get this test fixed? Or will that happen when we flip Window.prototype to WebIDL bindings? >+++ b/dom/tests/mochitest/general/test_interfaces.html >+var prefixedProperties = Could you please document that this is needed because runTest() thinks strings starting with "moz" and a capital letter are interface names? We still need codegen changes, in a followup, to not break this when we switch Window to actual WebIDL bindings, right? r=me with the above nits. This is awesome.

\o/

Backed out because this depended on bug 932309, which needed to be backed out. https://hg.mozilla.org/integration/mozilla-inbound/rev/2f97de1c0727

Backed out for making Linux32 opt mochitest-bc perma-orange. https://hg.mozilla.org/integration/mozilla-inbound/rev/08d4847dfd5c https://tbpl.mozilla.org/php/getParsedLog.php?id=30087228&tree=Mozilla-Inbound

With extended timeout for the failing test: https://hg.mozilla.org/integration/mozilla-inbound/rev/834b35ebbce7

Apparently a timeout of 4 still isn't enough. Upped it to 6. https://hg.mozilla.org/integration/mozilla-inbound/rev/66fe02c5c163 Am I understanding correctly that this test will get perpetually slower as we continue to add more properties? That smacks of very poor test design. I think the next time I see this test fail, I'm just going to disable it. This is already becoming a joke.

I think this regressed Bug 916851 rather badly. Verifying.

Regression: Mozilla-Inbound-Non-PGO - Dromaeo (CSS) - Ubuntu HW 12.04 - 2.7% decrease ------------------------------------------------------------------------------------- Previous: avg 4953.760 stddev 40.139 of 12 runs up to revision ba666d2dbb97 New : avg 4819.818 stddev 43.211 of 12 runs since revision 834b35ebbce7 Change : -133.942 (2.7% / z=3.337) Graph : http://mzl.la/1a5OLL4 Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ba666d2dbb97&tochange=834b35ebbce7

https://hg.mozilla.org/mozilla-central/rev/834b35ebbce7 https://hg.mozilla.org/mozilla-central/rev/c6ebee99e089

So yes, this did regress Bug 916851 about 50%.

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #11) > Apparently a timeout of 4 still isn't enough. Upped it to 6. > https://hg.mozilla.org/integration/mozilla-inbound/rev/66fe02c5c163 > > Am I understanding correctly that this test will get perpetually slower as > we continue to add more properties? That smacks of very poor test design. I > think the next time I see this test fail, I'm just going to disable it. This > is already becoming a joke. Unfortunately, this was not expected. Populating the variables view shouldn't take so much with all of those window properties. Afaik, we had some work done for improving the performance of vview. If the test fails again, please feel free to disable the test and file a bug - please CC me and vporof.

Is there more to document than the trap described in bug 943958 ? If this breaks an add-on, doesn't it have high chance to also break the web? :-(

Well, any sites that rely on this may already be broken in Chrome. I made a similar change before, and the only evidence of breakage I saw was a single site for a local club, and the page was broken in Chrome, which already had our behavior for that particular property.

(In reply to Andrew McCreight [:mccr8] from comment #20) > Well, any sites that rely on this may already be broken in Chrome. I made a > similar change before, and the only evidence of breakage I saw was a single > site for a local club, and the page was broken in Chrome, which already had > our behavior for that particular property. Oh yeah I had forgotten that it's already Chrome's behavior, my bad. Thanks!

> Is there more to document than the trap described in bug 943958 ? No. > doesn't it have high chance to also break the web? Maybe. > any sites that rely on this may already be broken in Chrome. Sort of. Chrome puts idl attributes on the window but methods on the prototype. We now put both on the window. The good news is that methods are just value props, so doing: var setTimeout = 5; works fine and sets it to 5. It's probably not optimized as well as a var with some other name, though. So you have to go out of your way to be broken by methods moving off the proto. For example, you have to be setting Window.prototype.setTimeout to to a function of your own or some such. See also bug 943065.

Chrome (or WebKit/Blink) just keeps pre-ES5 (or old IE) behavior (methods are on prototype, attributes are defined as own properties). It is not specific to the window object. I don't understand why the spec try to continue inventing new behaviors, breaking the compatibility, and diverging implementations rather than converging them.

> I don't understand why the spec try to continue inventing new behaviors In this case, see bug 943065 comment 6 for an example that necessitated the new behavior...

I'll also note that the spec explicitly documents why [Global] is specified the way it is: http://heycam.github.io/webidl/#Global

Added this to https://developer.mozilla.org/en-US/Firefox/Releases/28/Site_Compatibility Please feel free to edit/clarify the description.

Note that this broke IBM Rational Change (which was apparently also broken in Chrome/Safari). See http://forums.mozillazine.org/viewtopic.php?f=23&t=2802773

Adding a disabled flag as per Bug 976920.

Added back to the 31 site compat doc: https://developer.mozilla.org/en-US/Firefox/Releases/31/Site_Compatibility#DOM https://twitter.com/FxSiteCompat/status/477510724810334208