On the Dell site, when going to product pages, there is a menu on the left with "Configurations", "Features & Design", "Accessories & Services", etc. and information matching the selected item on the right side. See http://www.dell.com/us/p/xps-13-9333/pd?oc=dncwp1238h&model_id=xps-13-9333 for example. This works fine on release and beta, and not in aurora and nightly, where both the menu and the content are just gone (which, incidentally, makes it impossible to buy anything) Thanks to mozregression, I could pinpoint bug 932322 as the root cause of the breakage. I don't have more time to investigate whether bug 932322 actually broke something or if Dell is doing something wrong.

I see an error in the web console on that page that might be related? TypeError: $switchCTA.offset(...) is null jsmin.ashx:152 That line in the file is this: if($(".switchCTA").length>0){var $switchCTA=$(".switchCTA:visible");zombieStart=parseInt($switchCTA.offset().top)+parseInt($switchCTA.height());} This error does not show up on my release version of Firefox.

This seems to do with putting "onload" directly on the global. The exception in comment 1 seems to be in http://www1-cdn.dell.com/JS/default/jsmin.ashx?set=magPd&theme=default&cmp=1&version-13.11.11.1 which has a bunch of places that set bareword "onload" without declaring it as a var, but the behavior of that should not have changed (should set to null both before and after). It also has some "var onload", but they're all in functions...

This is moderately insane, actually. The page includes http://www1-cdn.dell.com/JS/default/jsmin.ashx?set=core&theme=default&cmp=1&version=13.11.11.1 which has at toplevel: var onload; then the script linked in comment 2 comes later and has a bunch of non-var-prefixed bareword things like this in functions: onload = { foo: bar }; doSomething(onload); So before bug 932322 this shadowed the "onload" on Window.prototype and it all worked. But now we see that the passed-in thing is not callable and per spec set window.onload to null on that bareword set. Then the doSomething() call doesn't work right. So I tried experimenting a bit as to what other UAs do. Here are the results: Chrome: Assigning non-callables to window.onload normally fails. However, "var onload" somehow manages to shadow the normal onload property, because Chrome actually puts that property on Window.prototype! In fact, it does that for all the on* properties I've tested so far, even though other properties (like window.performance) go directly on the Window object. Safari: Puts "onload" directly on the Window, and ignores "var onload" as expected, but generally allows assigning _any_ object, not just a callable, to any event handler DOM attribute. If the object is not callable, the event handler just seems to silently do nothing? Opera: Seems to have the same behavior as Gecko used to have: the on* properties are on the prototype and only allow callables. Note that this is Presto-based Opera, not Chromium-based. IE11: Seems to put "onload" on Window.prototype but allows assigning any object like Safari, so is OK for both reasons. IE9: Same as IE11. I'm going to post to public-script-coord with a note about this stuff, but how do we actually want to make this work for now? Seems like we should probably back out bug 932322 for the moment and then figure out what we actually want to implement and propose spec changes. :(

Or I guess we could hope that Dell is the only site doing this sort of thing, but I'm not sure I want to rely on that.

http://lists.w3.org/Archives/Public/public-script-coord/2013OctDec/0405.html fwiw.

Of all the different behaviours, I like Safari's the most. It seems less special-casey than treating any non-callable thing as null. I think I also prefer having all the Window members on the instance, rather than splitting them up between the instance and Window.prototype.

Safari's behavior requires adding a new kind of IDL type (possibly replacing [TreatNonCallableAsNull] callbacks?), right? A type just like a callback function, but that takes any object and is a no-op if the object is not callable?

Also, note that Safari still treats non-objects as null...

Sigh. So I wonder if treating non-objects as null is really required, or whether that was just the easiest thing to do to handle random values being assigned to onload while keeping the internal type being EventHandler (or whatever equivalent they would have had at the time).

I have no idea, but I'm not all that willing to experiment with changing something that every single UA does. One other note: Treating non-callables differently from callables is not hard to do in JS proper: just check that typeof(value) == "function". Treating all objects differently from all non-objects actually requires a lot more going out of one's way (though obviously not in Gecko's C++, where that's an easy test).

Is the backout of bug 932322 the only (and less risky) option here? If so we should definitely make this happen in Aurora so please nominate for landing.

It's not clear which is riskier. Backing out would get us back to a behavior we used to have, but it's not clear how much stuff has been added since that depends on the new behavior...

Talked to Peter. We're going to try the non-object thing on both m-c and aurora.

Just fixed up merge conflicts; no substantive changes, so I don't think this needs extra review

So there is one test failure: dom/imptests/html/html/webappapis/scripting/events/test_event-handler-spec-example.html This test comes down more or less to this: var div = document.createElement("div"); div.onclick = {}; var z = 0; div.addEventListener("click", function(e) { z = 1; }); div.onclick = function(e) { z = 2;} div.dispatchEvent(new Event("click")); assert(z == 2); We used to pass because the first onclick assignment didn't actually register an event listener (since it got set to null), so the z = 2 listener ran after the z = 1 listener. But now we add the listener at the first assignment, then update it in place, so z = 2 runs before z = 1. I checked other browsers, and WebKit/Blink seem to add a new listener on every onclick assignment, while IE11 has the behavior we would have with this patch. So I have no problem with the behavior change. The only question is how the test should be fixed. I can change the expected values, or I can replace that "{}" with "5" or something to make it continue to have the "set to null" behavior. Ms2ger, do you know what the intent of this test is? I guess ideally we'd fork the test and test it both ways....

I expanded the test: https://github.com/w3c/web-platform-tests/pull/506

Awesome, thanks! I'll just need to land the import of that with this patch...

Comment on attachment 8358468 [details] [diff] [review] With the test update rolled in Review of attachment 8358468 [details] [diff] [review]: ----------------------------------------------------------------- I suppose you're removing TreatNonCallableAsNull in a followup to minimize risk? ::: dom/bindings/Codegen.py @@ +11214,1 @@ > " ${argc}, ${argv}, rval.address())) {\n" Wrapping and whitespace end up a bit weird in the generated code here. Not sure how to make it better. ::: dom/imptests/html/html/webappapis/scripting/events/test_event-handler-spec-example.html @@ +7,5 @@ > +var objects = [{}, function() {}, new Number(42), new String()]; > +var primitives = [42, null, undefined, ""]; > +objects.forEach(function(object) { > + var t = async_test("Event handler listeners should be registered when they " + > + "are first set to a object value (" + an object value

> I suppose you're removing TreatNonCallableAsNull in a followup to minimize risk? No, I'm not removing it at all because we actually need it for Promise.... That's not a webidl spec issue, because Promise is not even being specced in WebIDL. > Wrapping and whitespace end up a bit weird Yeah. I could make it better by creating a separate if for the callGuard and making it "true" when we just want to always make the call, if you prefer... > an object value Good catch. ;)

https://hg.mozilla.org/integration/mozilla-inbound/rev/7770d2e633b1 with the imptest fix left to ms2ger.

Comment on attachment 8359379 [details] [diff] [review] Aurora patch with fixed commit message [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 932322 User impact if declined: Some sites are broken (e.g. dell.com) Testing completed (on m-c, etc.): Passes tests. Risk to taking this patch (and alternatives if risky): Risk is moderate, especially for web compat. It helps a bit that we're switching to an existing (IE11) behavior... The other option it trying to back out bug 932322, which seems riskier. String or IDL/UUID changes made by this patch: None.

fix works for me on firefox 29, linux X86_64.

Comment on attachment 8359379 [details] [diff] [review] Aurora patch with fixed commit message That this only needs to uplift to Aurora buys some time and it's the less risky option between landing this and doing a backout - approving.

Mike, can you please verify this is fixed for you in today's Nightly and tomorrow's Aurora?

It works for me.

Fix is not working for me on Aurora, Linux 32 Bit.

AShickur Rahman, your build doesn't have this fix. Note the "tomorrow" in comment 32.

ohh, yes, I mixed up with time zone.