Re: "var" declarations shadowing properties from Window.prototype

Travis Leithead wrote:
>> From: Boris Zbarsky [mailto:bzbarsky@MIT.EDU]
>>
>> On 8/12/12 5:29 PM, Brendan Eich wrote:
>>> Boris Zbarsky wrote:
>>>> Note that data in
>>>> http://lists.w3.org/Archives/Public/public-script-coord/2012JanMar/00
>>>> 33.html suggests that IE also implements the erratum to 5.1 we were
>>>> talking about up-thread.  Oh what a tangled web we weave.
>>> Yes, current thinking is that we should take the erratum that major JS
>>> engines already fixed, and include it in ES6. But this means we must
>>> do something different in WebIDL, probably make own global properties
>>> for window-implements-interface-inherited attributes and even
>> operations.
>>> And then (for strict mode) be careful about get-only accessors. This
>>> reminds me of [Replaceable], which was for non-writable but
>>> configurable data properties that var and function must be able to
>>> replace. It's kind of the opposite and only for accessors:
>>> var-captures-own-accessor-via-detection, or some such.
>>>
>>> Just recapping, tell me if I'm missing something.
>> The above sounds like a reasonable summary to me.  Certainly hits all the
>> high points of the discussion, with the addition that the GSP as currently
>> specced depends on the erratum sticking around.
>
> Allow me to also recap to ensure I understand this thread:
>
> The problem: a few [high profile] sites are using a coding practice that uses feature detection of the following pattern:
>     var [standardized property name] = window.[standardized property name] || window.[implementation-specific property name] || [etc.]

Usually window.[standardized property name] is last -- this matters below.

> Firefox is affected by this problem (e.g., the result of the var declaration is undefined)
> Chrome is not affected by this problem because their var creation algorithm checks the prototype chain for an existing property name

No, Chrome is not affected because their IDL bindings put inherited 
attributes on the global object as "own" accessors.

> IE10 is not affected by this problem because they define both "indexedDb" and "msIndexedDb" and the latter wins--otherwise they _would_ be affected by this problem.

That's true, but see above point about order of || terms.

> The reaction by this group is:
> * Don't change anything about ES5.1 var declaration and initialization (because we like the behavior, it works well with global scope pollutors in the prototype chain)

No. Rather, the erratum at 
https://bugs.ecmascript.org/show_bug.cgi?id=78 was already fixed by 
Mozilla, Google, Opera, and I believe IE and Apple. We think it's 
important for user-defined non-configurable properties on prototypes of 
the global object that var ignore such properties.

Instead, we think WebIDL must make a special case for inherited 
properties of the global object: make them "own".

Also, WebIDL-reliant spec authors must be careful not to define get-only 
accessors (readonly attributes) on the global object or interfaces from 
which it inherits. This leads to the idea that the global object 
defaults to [Replaceable] (which may be removed from WebIDL as explicit 
qualifier syntax) and [Unforgeable] must be used selectively for some 
few historic exceptions.

> * Change WebIDL so that any properties that would mixin to Window (or any ancestors that Window would inherit from) would instead be created directly on the global object (instead of on a prototype of the global).

Yes, but note this is sufficient to keep the fix for erratum 
https://bugs.ecmascript.org/show_bug.cgi?id=78 and not revert to ES5.1 spec.

>   Additionally, ensure that all of these properties are [Replaceable] meaning that if they are readonly (no setter), a [[Put]] request would instead create a data property of the same name in place of the pre-existing property.

Right.

> One side effect as I understand this, would be that:
>     var indexedDb = "test";
>     alert(indexedDb);
> would result in "test", and the original indexedDb property would be lost. Is this your understanding as well?

You bet. Who knows whether indexedDB (such a lovely name! snark) was not 
used as a var name (undeclared, even) in 2003 by some important site in 
Slovenia?

> Another side effect of this proposed change is that:
>     var onload = function(e) { ...}
> would actually assign the event handler (it does in Chrome today, not in Gecko/IE9/10).

Note that if you take away the 'var ' then Gecko at least assigns the 
event handler.

Per http://www.w3.org/TR/html5/browsers.html#the-window-object the 
onload, etc. attributes are read/write, so not (implicitly in a new 
world, and certainly not explicitly in the HMTL5 spec) [Replaceable]. So 
given the change to put inherited attributes on the global as "own" 
properties, 'var' can't matter. 'var' does not replace an existing own 
binding in ES5.1 or ES5.1 + the erratum fix.

> I was curious just how "bad" the currently reported bug actually is. I ran a query looking for use of var indexedDb and var requestAnimationFrame across our web data index (which is unfortunately about a year old). jQuery uses the requestAnimationFrame a lot--but their use is contained to the jQuery scope closure. Modernizer was also using the var indexedDb technique, but Modernizer also runs in a scope closure and thus is not affected by this issue. In fact, any major library (and a lot of other sites that have adopted the "module pattern") won't be affected by this at all.

We need a lot more testing to know how bad, but things are bad enough 
IMHO, and this looks like a recurring problem.

> So this begs the question: are we overreacting here? Personally, I'd like to avoid making a change to IE in this regard if I can avoid it. I'm not sure I'd recommend a change to WebIDL for this issue if we had found it in the wild--we might just expect sites to adjust their behavior instead (these affected high profile sites could arguably make an easy change to avoid this problem). The two side effects that immediately came to mind above could have worse impliciations than the current issue--especially the [Replaceable] change, since you inadvertently lose the original property in that case.

You get what the author wants, there is no unintended data loss.

Really, I urge you to consider how bad a plan it was (sorry I didn't 
flag it) to add readonly attributes to Window.prototype. That is a 
recipe for ongoing web compatibility breakage, whether 'var' is used or 
just bare assignment. HTML is not going to stop evolving and other specs 
will probably add predefined globals using "window implements NewInterface".

The ES5.1 erratum fix is important just for 'var' integrity in the face 
of Object.defineProperty on Object.prototype or any other window 
prototype chain object. We think we want it to stick in implementations, 
and to codify it in ES6.

The combination of the bad plan of window prototype chain extension, ad 
infinitum and with readonly attributes, and the ES5.1 erratum fix, 
creates an ongoing hazard. Since we want the ES5.1 erratum fix, and 
since the WebIDL global object prototype chain extension problem creates 
incompatibility even when 'var' is not used, I believe we should fix 
WebIDL to have a special case: flatten inherited attributes and 
operations onto the global as "own" properties.

/be

>
>
>
>> Travis Leithead wrote:
>>> Brendan Eich:
>>>> As noted, they started out that way 17 years ago. I think WebIDL and
>>>> interface-based method definition made onload, e.g., predefined on
>>>> window objects, or more recently on Window.prototype. Was this useful?
>>>> Was it intended specifically (for window, not just intended
>>>> generally due to WebIDL's uniform rules for binding its definitions in JS)?
>>> I don't think it provides any benefit.  Uniformity is the only reason
>>> the spec says they should be there, currently.
>> It does provide the monkey-patch benefit for "shared" interfaces (e.g., those shared by inheritance). At the present time, the only one I can think of that [will] act like this is EventTarget (IE10 hasn't yet implemented this hierarchy change).
>>
>> Do you think it's worth another exception (to the exception whereby inherited properties are flattened to be "own" on the global) that adds EventTarget.prototype to window objects' prototype chains? Presumably in front of (closer to the head of the global object itself) the GSP.
>
> Possibly, since we we're talking about exceptions for exceptions :) However, I'd like to consider not making a change at all given the consequences as I understand them.

Received on Wednesday, 15 August 2012 21:08:55 UTC