Real World Func Decl in Block Scope Breakages

Brian Terlson Brian.Terlson at microsoft.com
Wed Dec 26 16:12:51 PST 2012


Hi Brendan,

Responses inline.

> -----Original Message-----
> From: Brendan Eich [mailto:brendan at mozilla.com]
> Sent: Wednesday, December 26, 2012 3:27 PM
> To: Brian Terlson
> Cc: es-discuss at mozilla.org; Charles McCathie Nevile
> Subject: Re: Real World Func Decl in Block Scope Breakages
> 
> Brian Terlson wrote:
> > 20 sites, however, will likely be broken by this change in some way.
> > There is also a chance that the tool used to identify breakages has
> > missed some code that will breka.
> >
> > Below are some examples of code on the web today that will be broken.
> > For each I include a snippet of code that is heavily edited in an
> > attempt to convey the pattern used and the developer intent. I also
> > attempt to identify what functionality will actually be broken.
> >
> 
> Thanks for the snippets. Some questions interspersed below.
> 
> > Most of the breakages occur in non-library code, with two exceptions:
> > qTip 1.0, and thickbox 3.
> >
> > # http://ninemsn.com.au
> >
> > RenderModal = function () {
> >
> > if (x) { // is an array of shortcuts that can be added. Is statically
> > non-empty.
> >
> > function K() {
> >
> > // process the array of shortcuts in some way
> >
> > }
> >
> > }
> >
> > K();
> >
> > };
> >
> 
> Since x evaluates as truthy no matter what (even an empty array is an object,
> therefore truthy), this is equivalent to
> 
> RenderModal = function () {
> 
> function K() {
> 
> // process the array of shortcuts in some way
> 
> }
> 
> K();
> 
> };
> 
> 
> but of course it lies outside of ES6's function-in-block semantics, yet still in
> the intersection of Firefox and IE/IE-emulating-browsers.
> 
> > # http://yandex.ru
> >
> > if (_ycssjs("BaH0Fmmo2Sg24lRmTPrK0B8qpaA")) {
> >
> > function cp(g, c, d) { /* ...*/ }
> >
> > function csh_ifgsid(c, b) { /*...*/ } }
> >
> > // ... thousands of lines of code ...
> >
> 
> This "then" block presumably closes here, before:
> 
> > if (_ycssjs("rO+QIoSf2L0NwDn6vjJjy+27nxI")) {
> >
> > function news() {
> >
> > csh_if_gsid();
> >
> > }
> >
> > }
> >
> > if (_ycssjs("YRPF0QVjJmhRiKRu6cvi3YXqYo8")) {
> >
> > // ... bunch of stuff ...
> >
> > cp();
> >
> > }
> >
> 
> Somehow the "if" conditions are logically connected such that no calls to an
> undefined identifier occur! Blech.
> 
> > ## Scenario
> >
> >
> > Unknown
> >
> 
> Indeed! Cc'ing Chaals in case he can shed light.
> 
> > #
> > http://g.espncdn.com/nfl-primetime-
> payoff/en/module/entry?matchupid=47
> > 8
> >
> > if (isIE && isWin) {
> >
> > // ...
> >
> > } else {
> >
> > function JSGetSwfVer(i){
> >
> > // ...
> >
> > }
> >
> > }
> >
> > function checkFlash(myRev) {
> >
> > // ...
> >
> > if (isIE && isWin) {
> >
> > // ...
> >
> > } else {
> >
> > aV = JSGetSwfVer(rV);
> >
> > }
> >
> > }
> >
> > ## Scenario
> >
> > Can't find a page which uses this code.
> >
> 
> Can you say how many of the 20 cases involve actual live code using the
> function-in-pre-ES6-block code?

Just going by the snippets that have a known scenario rather than unknown, looks like 10/20. For the others, I do not know for sure whether the snippet is dead code or not. If you have any suggestions for how I could determine this more easily than heavily using the site under instrumentation I can look into collecting that data before the Jan meeting.

> 
> > # http://www.t-online.de
> >
> > if (!Adition_Environment) {
> >
> > var Adition_Environment = (function () {
> >
> > var _this = {};
> >
> > // ...
> >
> > _this.getPrf = function (cuId) {
> >
> > var prf = "";
> >
> > try {
> >
> > prf = Adition_Prfstr(cuId);
> >
> > } catch (e) { }
> >
> > return prf;
> >
> > };
> >
> > // ...
> >
> > })();
> >
> > }
> >
> > // snip 1k lines
> >
> > if (typeof Adition_Prfstr == "undefined") {
> >
> > function Adition_Prfstr(ADITION_CONTENTUNIT_ID) {
> >
> > // ...
> >
> > }
> >
> > }
> >
> > ## Scenario
> >
> > Required to display advertisements on the page.
> >
> 
> Heinous.
> 
> > # http://manormystery.com
> >
> > if (!window._ate) {
> >
> > window._ate = { /* ... */ };
> >
> > function addthis_open() { /* ... */ } } else { _ate.inst++; }
> >
> >
> 
> Did you snip the closing brace for the "then" block left open, the one for "if
> (!window._ate)"?
> 
> > if (_atc.abf) {
> >
> > addthis_open(document.getElementById("ab"), "emailab",
> > window.addthis_url || "[URL]", window.addthis_title || "[TITLE]"); }
> >
> > ## Scenario
> >
> > Social sharing popups broken at least. This may be a general utility
> > used in a number of places.
> >
> 
> On the other hand, the snippet seems like a mis-coding of
> 
> if (!window._ate) {
> 
> window._ate = { /* ... */ };
> 
> }
> 
> function addthis_open() { /* ... */ } } else { _ate.inst++; }
> 
> 
> if (_atc.abf) {
> 
> addthis_open(document.getElementById("ab"), "emailab",
> window.addthis_url || "[URL]", window.addthis_title || "[TITLE]"); }
> 
> ...
> }
> 
> I'd try to evangelize the owner of this code.
> 
> > # http://manhunt.net (NSFW, DO NOT VISIT AT WORK)
> >
> > /** @todo isFlashInstalled() should probably be gotten by include from
> > js/cmmn/flashDetect.js or upfunc.js. */
> >
> > if (typeof isFlashInstalled == 'undefined') {
> >
> > function isFlashInstalled() {
> >
> > var requiredMajorVersion = 6; // Major version of Flash required
> >
> > var requiredMinorVersion = 0; // Minor version of Flash required
> >
> > var requiredRevision = 0; // Minor version of Flash required
> >
> > var s = new SWFObject();
> >
> > if (!s) return false;
> >
> > var version = s.installedVer;
> >
> > if (!version) return false;
> >
> > return (version.major >= requiredMajorVersion && version.minor >=
> > requiredMinorVersion && version.rev >= requiredRevision);
> >
> > }
> >
> > }
> >
> > isFlashInstalled();
> >
> 
> Here is the simplest example of a pre-ES6 Firefox/IE+clones
> intersection: function-in-block with hoisting to top of outer function or global
> scope *and* SpiderMonkey's assignment-like runtime binding both work the
> same, provided the use(s) of the function's name all occur after any such
> assignment (in the SpiderMonkey in Firefox scenario).
> 
> There's no control flow relation in general, e.g. dominance. However in this
> case the "if (typeof isFlashInstalled == 'undefined')" head could, to a
> sufficiently smart CFG analyzer, imply that isFlashInstalled() post-dominates
> some kind of "definition" of that name.
> 
> Ok, enough so far to say a few things:
> 
> 1. We have time and (among the bigs involved in ES6) try evangelization.
> Evangelization can be crowd-sourced too, and can be more effective when
> done that way.
> 
> 2. We have some terrible choices, even if ES6's function-in-block semantics
> require "use strict". That leaves the above mess a de-facto standard which
> has never been specified. Specifying the sloppy-mode intersection is going to
> be ugly and a time-sink.
> 
> 3. The carrot of function-in-block may be sweet -- we have evidence in all the
> good sites your crawl found that are not the insane 20 -- but whether "use
> strict"; required before all such ES6 function-in-block uses would be a stick
> remains to be investigated. Did you check for "use strict" or actual enabled-
> in-IE10 strict mode among the sites you found that declare functions in
> blocks?

Programmers do seem to want func decls in block scope a lot more than I expected, although I wouldn't necessarily call all non-breaking instances of it I found "sane" (eg. the decls I found in while/for statements...), just less insane :)

I didn't check for "use strict". I will work on capturing this data at least in time for the Jan meeting.

As far as IE doc modes, 15/19 are in standards mode (Zoompanel.com, Kankan.com, 163.com, and Verizon.com run in some compat mode). Didn't check the NSFW site.

> 
> /be
> 
> 
> 




More information about the es-discuss mailing list