407314 - Stop polluting namespace of XBL constructors and event handlers

Status: RESOLVED WONTFIX

(Core :: XBL, defect)

Description

[Wladimir Palant]

Currently, the following code in an XBL binding on a XUL element will not work as expected:

<constructor>
  var opener = top.opener;
</constructor>

Reason: XUL elements have a property |top|, and when we refer to |top| we get this property here instead of |window.top|. The fact that all properties of the node are "imported" as local variables in constructors and event handlers (but not methods) is totally non-obvious and undocumented. In the end, the XBL constructor is the counterpart of a class constructor, but a class constructor doesn't show this behavior:

function Class() {
  alert(top);   // Will show "[Window]"
}
Class.prototype = {top: ""};
new Class();

Also, the fact that this behavior is inconsistent between event handlers and methods doesn't make things better. I suspect that nobody ever used this feature consciously, the two occurrences in the Firefox code are obviously bugs, same with the one occurrence in TomTom HOME. IMO this means that this behavior should be treated as a bug, it needs to be fixed.

Attachment 289711 [details] [diff] is a partial patch.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is invalid for event hanlers, no?  Event handlers in general extend the scope chain, I thought, even outside XBL.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'd be worried about fixing this. Are you sure there aren't bindings out there that depend on this?

Comment 3

[Wladimir Palant]

(In reply to comment #1)
> This is invalid for event hanlers, no?  Event handlers in general extend the
> scope chain, I thought, even outside XBL.

I checked this - you are right, however this is only true for inline event handlers (guess you can always learn something new about HTML). This kind of makes sense given the way these event handlers are specified. Any handler attached in the "usual" way has a normal scope however.

(In reply to comment #2)
> I'd be worried about fixing this. Are you sure there aren't bindings out there
> that depend on this?

I guess checking this automatically for AMO extensions shouldn't be too hard. I definitely didn't see this feature documented anywhere (neither on MDC nor external sites).

Comment 4

[Boris Zbarsky [:bzbarsky]]

XBL is now disabled in Firefox (Bug 1583314) and is in the process of being removed from Gecko (Bug 1566221), so closing bugs requesting changes to its implementation as wontfix.