New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annex B.3.3 semantics may leak arbitrary values via the synthesized outer var assignment #348

Closed
syg opened this Issue Feb 3, 2016 · 36 comments

Comments

Projects
None yet
9 participants
@syg
Member

syg commented Feb 3, 2016

Firefox ran into a block-scope function compat issue with the Rackspace webmail app that boils down to the following pattern:

{
  f = 42;
  function f() { } 
}
print(f); // What is this supposed to be?

If I'm reading the spec correctly, B.3.3.1's step 1.a.ii.3.e, B.3.3.2's step 4.b.i.2.c.v, and B.3.3.3's step 1.d.ii.7.b.v all say to:

  1. Look up the value of f in the block scope.
  2. Assign the looked up value to @var.f in the var scope.

But that means arbitrary values can leak out via this assignment, as in the above test case. The sensible thing here would be that the synthesized Annex B assignment always uses the function object as the RHS. Edge seems to do this.

Is this a spec bug or am I missing something?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 3, 2016

Member

The current language is intentional.

The reason that B.3.3 is "leaking" the current block value of 'f' is that the ES6 block instantiateon semantics doesn't independently capture the function object it creates for 'f'

Based on my understanding at the time B.3.3 was originally authored, the example you give in this issue would not have been consider interoperable code. In other words, legacy browsers would not all produce the same result of that code, so ES6 did not have to support it in a backwards compatible manner.

Most non-spider monkey based-browers would be printed 42 because the block level function declaration would have been hosted to the top of the surrounding top-level scope and initialize to the function object. During block evaluation block level assignment would have assigned 42 to that top level binding and the function declaration would be ignore. The print state would output 42.

My understanding of the legacy spider monkey semantics for that code is that it did not hoist the block declaration level declaration to the surrounding top-level cope level when instantiating the top[-level. When evaluating the block, assignment to f within the block would then be an assignment to an unbound variable, so it created a property name 'f' on the global object with the value 42. When it evaluated the block level function declaration it would dynamically create a var binding for 'f' in the surrounding top-level scope (dynamically shadowing the the global property 'f', if this code is within a function body) and initialize it to the function object. The print statement output is the toString of the function body.

QED, not interoperable. If that understanding of the legacy spider monkey semantics is wrong then we need to reexamine exactly what is the FiB legacy intersection semantics.

Member

allenwb commented Feb 3, 2016

The current language is intentional.

The reason that B.3.3 is "leaking" the current block value of 'f' is that the ES6 block instantiateon semantics doesn't independently capture the function object it creates for 'f'

Based on my understanding at the time B.3.3 was originally authored, the example you give in this issue would not have been consider interoperable code. In other words, legacy browsers would not all produce the same result of that code, so ES6 did not have to support it in a backwards compatible manner.

Most non-spider monkey based-browers would be printed 42 because the block level function declaration would have been hosted to the top of the surrounding top-level scope and initialize to the function object. During block evaluation block level assignment would have assigned 42 to that top level binding and the function declaration would be ignore. The print state would output 42.

My understanding of the legacy spider monkey semantics for that code is that it did not hoist the block declaration level declaration to the surrounding top-level cope level when instantiating the top[-level. When evaluating the block, assignment to f within the block would then be an assignment to an unbound variable, so it created a property name 'f' on the global object with the value 42. When it evaluated the block level function declaration it would dynamically create a var binding for 'f' in the surrounding top-level scope (dynamically shadowing the the global property 'f', if this code is within a function body) and initialize it to the function object. The print statement output is the toString of the function body.

QED, not interoperable. If that understanding of the legacy spider monkey semantics is wrong then we need to reexamine exactly what is the FiB legacy intersection semantics.

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 3, 2016

Member

@allenwb Are we in agreement that B.3.3 as it is currently would print 42?

So we have an annex whose sine qua non is maintaining backwards compatbility, but whose language also definitely breaks backwards compatibility because there was no consistent backwards compatibility. Great.

What we have now is a big app that breaks with the current spec language. Edge doesn't implement the spec (if it is supposed to print 42). Additionally, I think that printing 42 would be very surprising, and if we're going to implement bananas semantics in the name of backwards compatibility that doesn't exist in a consistent fashion anyways, I'd rather do the intuitive and (thus far) less breaking thing.

Member

syg commented Feb 3, 2016

@allenwb Are we in agreement that B.3.3 as it is currently would print 42?

So we have an annex whose sine qua non is maintaining backwards compatbility, but whose language also definitely breaks backwards compatibility because there was no consistent backwards compatibility. Great.

What we have now is a big app that breaks with the current spec language. Edge doesn't implement the spec (if it is supposed to print 42). Additionally, I think that printing 42 would be very surprising, and if we're going to implement bananas semantics in the name of backwards compatibility that doesn't exist in a consistent fashion anyways, I'd rather do the intuitive and (thus far) less breaking thing.

@claudepache

This comment has been minimized.

Show comment
Hide comment
@claudepache

claudepache Feb 3, 2016

Contributor

Firefox ran into a block-scope function compat issue with the Rackspace webmail app that boils down to the following pattern:

{
  f = 42;
  function f() { } 
}
print(f); // What is this supposed to be?

That prints 42 in Chrome and Safari.

However, the test case in the Mozilla bug report is more akin to the following one:

{
  if (!this.f) {
    f = 42;
  }
  function f() { } 
}
print(f); // What is this supposed to be?

which does print consistently function f() { } in all mainstream browsers.

Contributor

claudepache commented Feb 3, 2016

Firefox ran into a block-scope function compat issue with the Rackspace webmail app that boils down to the following pattern:

{
  f = 42;
  function f() { } 
}
print(f); // What is this supposed to be?

That prints 42 in Chrome and Safari.

However, the test case in the Mozilla bug report is more akin to the following one:

{
  if (!this.f) {
    f = 42;
  }
  function f() { } 
}
print(f); // What is this supposed to be?

which does print consistently function f() { } in all mainstream browsers.

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 3, 2016

Member

@claudepache In Firefox Nightly, for the first time that snippet runs, it prints 42. Step by step below:

{
  if (!this.f) { // Annex B synthesized an undefined global var binding
    f = 42; // Assigns to the block-local 'f' introduced by the FunctionDeclaration below due to hoisting
  }
  function f() { } // Assigns the block-local value of 'f' (42) to the global var binding of 'f'
}
print(f); // Prints the global 'f', which is now 42.
Member

syg commented Feb 3, 2016

@claudepache In Firefox Nightly, for the first time that snippet runs, it prints 42. Step by step below:

{
  if (!this.f) { // Annex B synthesized an undefined global var binding
    f = 42; // Assigns to the block-local 'f' introduced by the FunctionDeclaration below due to hoisting
  }
  function f() { } // Assigns the block-local value of 'f' (42) to the global var binding of 'f'
}
print(f); // Prints the global 'f', which is now 42.
@claudepache

This comment has been minimized.

Show comment
Hide comment
@claudepache

claudepache Feb 3, 2016

Contributor

In Firefox Nightly, for the first time that snippet runs, it prints 42

FWIW, in Chrome 49 Beta, it has also changed relatively to the previous version: my testcase now returns 42.

Contributor

claudepache commented Feb 3, 2016

In Firefox Nightly, for the first time that snippet runs, it prints 42

FWIW, in Chrome 49 Beta, it has also changed relatively to the previous version: my testcase now returns 42.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 3, 2016

Member

@syg So we have an annex whose sine qua non is maintaining backwards compatbility, but whose language also definitely breaks backwards compatibility because there was no consistent backwards compatibility. Great.

Exactly. Read all of B.3.3 starting from the beginning. Legacy browsers did not implement function declarations in blocks using a common semantics. In particular, Spider Monkey's semantics were quite different from other browsers. Because of this only a subset of code that used functions declaration in blocks worked interoperably in all browsers. Annex B.3.3 purpose is to define a semantics that is backwards compatible with that subset.

You may also want to research the TC39 notes and presentations that led to the creation of B.3.3

Member

allenwb commented Feb 3, 2016

@syg So we have an annex whose sine qua non is maintaining backwards compatbility, but whose language also definitely breaks backwards compatibility because there was no consistent backwards compatibility. Great.

Exactly. Read all of B.3.3 starting from the beginning. Legacy browsers did not implement function declarations in blocks using a common semantics. In particular, Spider Monkey's semantics were quite different from other browsers. Because of this only a subset of code that used functions declaration in blocks worked interoperably in all browsers. Annex B.3.3 purpose is to define a semantics that is backwards compatible with that subset.

You may also want to research the TC39 notes and presentations that led to the creation of B.3.3

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Feb 3, 2016

Contributor

Was Annex B.3.3 designed under the assumption that if legacy engines don't agree on a non-standard feature, then that feature may be formally specified in a way that requires changes to the behavior of some engines?

Clearly we can still "break the web" in such a scenario.

Contributor

zenparsing commented Feb 3, 2016

Was Annex B.3.3 designed under the assumption that if legacy engines don't agree on a non-standard feature, then that feature may be formally specified in a way that requires changes to the behavior of some engines?

Clearly we can still "break the web" in such a scenario.

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Feb 3, 2016

Member

@zenparsing How is that breaking the web?

Member

michaelficarra commented Feb 3, 2016

@zenparsing How is that breaking the web?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 3, 2016

Member

@zenparsing
Code that requires a specific brower is not part of the interoperable web.

Member

allenwb commented Feb 3, 2016

@zenparsing
Code that requires a specific brower is not part of the interoperable web.

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Feb 3, 2016

Contributor

@michaelficarra I was thinking that the failure scenarios might be really complex. For instance, a program whose control path is dependent on the engine for other reasons, causing it to only select the semantic variant that is not chosen as the standard, for that browser only.

But perhaps I'm misunderstanding the situation with B.3.3 (which I'm personally curious about for future spec-lessons). Are the sites that have been broken by B.3.3, broken because they relied upon non-interoperable semantics? Or because B.3.3 turns out to be non-interoperable?

Contributor

zenparsing commented Feb 3, 2016

@michaelficarra I was thinking that the failure scenarios might be really complex. For instance, a program whose control path is dependent on the engine for other reasons, causing it to only select the semantic variant that is not chosen as the standard, for that browser only.

But perhaps I'm misunderstanding the situation with B.3.3 (which I'm personally curious about for future spec-lessons). Are the sites that have been broken by B.3.3, broken because they relied upon non-interoperable semantics? Or because B.3.3 turns out to be non-interoperable?

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 3, 2016

Member

Let's not derail the conversation on what it means to "break the web" or the origins of Annex B and its intentions. I did not mean to bring those into question, and I apologize for previous snark.

Empirically, based on comments from @claudepache, Chrome and Firefox did adhere to legacy SM semantics until recently, and Edge still does. In light of the Rackspace breakage, I would consider this to be a compat problem.

Member

syg commented Feb 3, 2016

Let's not derail the conversation on what it means to "break the web" or the origins of Annex B and its intentions. I did not mean to bring those into question, and I apologize for previous snark.

Empirically, based on comments from @claudepache, Chrome and Firefox did adhere to legacy SM semantics until recently, and Edge still does. In light of the Rackspace breakage, I would consider this to be a compat problem.

@claudepache

This comment has been minimized.

Show comment
Hide comment
@claudepache

claudepache Feb 3, 2016

Contributor

Empirically, based on comments from @claudepache, Chrome and Firefox did adhere to legacy SM semantics until recently

No, Chrome and Firefox had wildly different semantics (as explained earlier by @allenwb), that fortuitously happen to yield the same final result for that particular test case.

Contributor

claudepache commented Feb 3, 2016

Empirically, based on comments from @claudepache, Chrome and Firefox did adhere to legacy SM semantics until recently

No, Chrome and Firefox had wildly different semantics (as explained earlier by @allenwb), that fortuitously happen to yield the same final result for that particular test case.

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 3, 2016

Member

@claudepache Thanks for the clarification.

Moving forward, can other implementors (@littledan and @bterlson) chime in here on whether they think this is a legit compat issue? Currently Firefox is the only browser for which Rackspace breaks AFAICT.

Member

syg commented Feb 3, 2016

@claudepache Thanks for the clarification.

Moving forward, can other implementors (@littledan and @bterlson) chime in here on whether they think this is a legit compat issue? Currently Firefox is the only browser for which Rackspace breaks AFAICT.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 3, 2016

Member

@syg
Can you show us the actual rackspace code snippet?

Member

allenwb commented Feb 3, 2016

@syg
Can you show us the actual rackspace code snippet?

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 3, 2016

Member

Rackspace's webmail app uses a function L(s) to perform localization. They have this code:

if (wack && wack.ComboLoader && !wack.ComboLoader.isLoaded("webmail-js-framework-dist")) {
  // ...
  if (wack && wack.ComboLoader && !wack.ComboLoader.isLoaded("webmail-js-wack-combo")) {
  // ...
    if (!window.L) {
      L = function(b) {
        // fallback
      };
    }
  }
  // ...
  function L(f) {
    // real impl
  }
}
L("foo");

On Firefox, L("foo") calls the first function. On Edge and Chromium, it calls the second function.

Member

syg commented Feb 3, 2016

Rackspace's webmail app uses a function L(s) to perform localization. They have this code:

if (wack && wack.ComboLoader && !wack.ComboLoader.isLoaded("webmail-js-framework-dist")) {
  // ...
  if (wack && wack.ComboLoader && !wack.ComboLoader.isLoaded("webmail-js-wack-combo")) {
  // ...
    if (!window.L) {
      L = function(b) {
        // fallback
      };
    }
  }
  // ...
  function L(f) {
    // real impl
  }
}
L("foo");

On Firefox, L("foo") calls the first function. On Edge and Chromium, it calls the second function.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 4, 2016

Member

@syg
Is that global code, or is it within a function body?

Member

allenwb commented Feb 4, 2016

@syg
Is that global code, or is it within a function body?

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 4, 2016

Member

@allenwb It is global code.

Member

syg commented Feb 4, 2016

@allenwb It is global code.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 4, 2016

Member

Note that the ES6 spec. did not specify a how to handle FiBs contained in script code. That was an oversight, but important to note because the Rackspace code would work fine (and interoperably) if it was contained within a function body as specified in ES6. So the problem we are seeing here presumably comes from the new ES2015 section B.3.3.2 and possibly from Edge/Chromiums implementation of the ES6 unspecified functionality. In a function, window.L would not be alias to the synthetic declaration introduced to host the FiB to the top-level.

This probably means that more thought is needed about B.3.3.2

Member

allenwb commented Feb 4, 2016

Note that the ES6 spec. did not specify a how to handle FiBs contained in script code. That was an oversight, but important to note because the Rackspace code would work fine (and interoperably) if it was contained within a function body as specified in ES6. So the problem we are seeing here presumably comes from the new ES2015 section B.3.3.2 and possibly from Edge/Chromiums implementation of the ES6 unspecified functionality. In a function, window.L would not be alias to the synthetic declaration introduced to host the FiB to the top-level.

This probably means that more thought is needed about B.3.3.2

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Feb 4, 2016

Member

Sorry I missed the ping above. Yes this is definitely worrying from a compat perspective. If everyone used to behave interoperabley here then we should reflect that in the spec. Sounds like that's the case and it's just a bug in B.3.3.2. Can someone double-check JSC semantics just to make sure we're all agreed that Rackspace should work?

Member

bterlson commented Feb 4, 2016

Sorry I missed the ping above. Yes this is definitely worrying from a compat perspective. If everyone used to behave interoperabley here then we should reflect that in the spec. Sounds like that's the case and it's just a bug in B.3.3.2. Can someone double-check JSC semantics just to make sure we're all agreed that Rackspace should work?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 4, 2016

Member

Here is my understanding of what B.3.3.2 says should happen:

  1. GlobalDeclarationInstantiation for the Script will create a GlobalFunctionBind for L and initialize it to undefined. That binding is represented as a property of the global object
  2. BlockDeclarationInstantiation for the Block that is the then clause body of the 2nd IfStatement creates a lexical binding for L in the block environment and initialize it to the "real impl" function body
  3. !window.L will evaluate to true because the value of window.L is undefined.
  4. the "fallback" function object will be assign to the block level binding of L;
  5. at the point of the FunctionDeclaration for "real iml" the value of the block level L is accessed and assigned to the global variable L
  6. The "fallback" function is called with argument ("foo")
Member

allenwb commented Feb 4, 2016

Here is my understanding of what B.3.3.2 says should happen:

  1. GlobalDeclarationInstantiation for the Script will create a GlobalFunctionBind for L and initialize it to undefined. That binding is represented as a property of the global object
  2. BlockDeclarationInstantiation for the Block that is the then clause body of the 2nd IfStatement creates a lexical binding for L in the block environment and initialize it to the "real impl" function body
  3. !window.L will evaluate to true because the value of window.L is undefined.
  4. the "fallback" function object will be assign to the block level binding of L;
  5. at the point of the FunctionDeclaration for "real iml" the value of the block level L is accessed and assigned to the global variable L
  6. The "fallback" function is called with argument ("foo")
@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 4, 2016

Member

I can confirm that Chrome's semantics changed here, and it was due to the new ES2015 flags turning on. My reading of the spec text matches @allenwb 's, and I can see how @syg 's suggestion would give the website its expected behavior. The suggestion seems reasonable to implement as well.

What I'm not sure about is why the website has written the code this way. Seems like the conditional definition is post-dominated by the unconditional definition. Is this the result of some sort of uncontrolled code concatenation (as we've seen in some other problematic cases for Annex B 3.3)? Do we have any other information about how common this is?

Member

littledan commented Feb 4, 2016

I can confirm that Chrome's semantics changed here, and it was due to the new ES2015 flags turning on. My reading of the spec text matches @allenwb 's, and I can see how @syg 's suggestion would give the website its expected behavior. The suggestion seems reasonable to implement as well.

What I'm not sure about is why the website has written the code this way. Seems like the conditional definition is post-dominated by the unconditional definition. Is this the result of some sort of uncontrolled code concatenation (as we've seen in some other problematic cases for Annex B 3.3)? Do we have any other information about how common this is?

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 4, 2016

Member

Note that such a change as @syg is suggesting would have further implications beyond conflicts with variable setting, for example:

{
  assert(f() == 2);
  function f() { return 1; }
  assert(f() == 1);
  function f() { return 2; }
  assert(f() == 2);
}

I think this agrees with Firefox legacy semantics, but Chrome legacy semantics as well as Annex B 3.3 semantics would be that f() == 2 in all cases.

Member

littledan commented Feb 4, 2016

Note that such a change as @syg is suggesting would have further implications beyond conflicts with variable setting, for example:

{
  assert(f() == 2);
  function f() { return 1; }
  assert(f() == 1);
  function f() { return 2; }
  assert(f() == 2);
}

I think this agrees with Firefox legacy semantics, but Chrome legacy semantics as well as Annex B 3.3 semantics would be that f() == 2 in all cases.

@claudepache

This comment has been minimized.

Show comment
Hide comment
@claudepache

claudepache Feb 4, 2016

Contributor

Note that such a change as @syg is suggesting would have further implications beyond conflicts with variable setting

@littledan: I have a different interpretation:

{
  assert(f() == 2); // use the binding of the inner scope
  function f() { return 1; } // modify the binding of the global scope
  assert(f() == 2); // use the unmodified binding of the inner scope
  function f() { return 2; }
  assert(f() == 2);
}
Contributor

claudepache commented Feb 4, 2016

Note that such a change as @syg is suggesting would have further implications beyond conflicts with variable setting

@littledan: I have a different interpretation:

{
  assert(f() == 2); // use the binding of the inner scope
  function f() { return 1; } // modify the binding of the global scope
  assert(f() == 2); // use the unmodified binding of the inner scope
  function f() { return 2; }
  assert(f() == 2);
}
@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 4, 2016

Member

I have yet a 3rd interpretation. Since backing off early errors for FiB is not yet agreed upon or specced (I'll bring it up next meeting), we have leeway here. The spirit of backing off on early errors for FiB is that the last function wins, so semantically it'd be equivalent to previous declarations not being there at all. In other words, in the example from @littledan, the first declaration of f modifies neither the local nor the global binding.

{
  assert(f() == 2); // use the binding of the inner scope
  function f() { return 1; } // is a nop
  assert(f() == 2); // use the unmodified binding of the inner scope
  function f() { return 2; } // synthesizes an Annex B assignment to the global scope
  assert(f() == 2);
}
Member

syg commented Feb 4, 2016

I have yet a 3rd interpretation. Since backing off early errors for FiB is not yet agreed upon or specced (I'll bring it up next meeting), we have leeway here. The spirit of backing off on early errors for FiB is that the last function wins, so semantically it'd be equivalent to previous declarations not being there at all. In other words, in the example from @littledan, the first declaration of f modifies neither the local nor the global binding.

{
  assert(f() == 2); // use the binding of the inner scope
  function f() { return 1; } // is a nop
  assert(f() == 2); // use the unmodified binding of the inner scope
  function f() { return 2; } // synthesizes an Annex B assignment to the global scope
  assert(f() == 2);
}
@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Feb 4, 2016

Member

@syg
In the Rackspace code, is there any reference to the global L in the //... statement sequence?

If there isn't then the check of window.L and the setting of the "fallback" function seems completely unnecessary from a legacy compat perspective. As prior to any ES6 work, all major browsers would have ultimately set global L to the "real impl" function.

If this is a work around because of some recent ES6-related browser changes maybe it doesn't represent a legacy compat. issue.

If this is old code, then maybe it is just one-of junk code that isn't indicative of a commonly used legacy pattern that we need to worry about.

Also, hopefully somebody has told Rackspace how to change their code to avoid this problem.

Member

allenwb commented Feb 4, 2016

@syg
In the Rackspace code, is there any reference to the global L in the //... statement sequence?

If there isn't then the check of window.L and the setting of the "fallback" function seems completely unnecessary from a legacy compat perspective. As prior to any ES6 work, all major browsers would have ultimately set global L to the "real impl" function.

If this is a work around because of some recent ES6-related browser changes maybe it doesn't represent a legacy compat. issue.

If this is old code, then maybe it is just one-of junk code that isn't indicative of a commonly used legacy pattern that we need to worry about.

Also, hopefully somebody has told Rackspace how to change their code to avoid this problem.

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 4, 2016

Member

@allenwb The Rackspace code is long and minified, but from my glancing at it the fallback L() function is used for some day-of-the-week localization during the startup sequence. That's just an educated guess, though.

I will file a bug with Rackspace about their code, I'm in agreement that it is weird code. At the same time, the behavior that it manifested is also weird.

Member

syg commented Feb 4, 2016

@allenwb The Rackspace code is long and minified, but from my glancing at it the fallback L() function is used for some day-of-the-week localization during the startup sequence. That's just an educated guess, though.

I will file a bug with Rackspace about their code, I'm in agreement that it is weird code. At the same time, the behavior that it manifested is also weird.

@jon-armstrong

This comment has been minimized.

Show comment
Hide comment
@jon-armstrong

jon-armstrong Feb 8, 2016

I'm one of the Rackspace Webmail developers, and I can confirm that the "fallback" code is part of some very old framework code that was at some point separated from the main application as a dependency and forgotten about:

    if (!window.L) {
      L = function(b) {
        // fallback
      };
    }

I have no idea what it was originally used for, but I'm certain ES6 did not exist when this code was written. Based on the discussion of B.3.3 in this issue, the change in behavior makes sense. We're in the process of removing it from our code base (release date TBD). Thanks for bringing it to our attention.

jon-armstrong commented Feb 8, 2016

I'm one of the Rackspace Webmail developers, and I can confirm that the "fallback" code is part of some very old framework code that was at some point separated from the main application as a dependency and forgotten about:

    if (!window.L) {
      L = function(b) {
        // fallback
      };
    }

I have no idea what it was originally used for, but I'm certain ES6 did not exist when this code was written. Based on the discussion of B.3.3 in this issue, the change in behavior makes sense. We're in the process of removing it from our code base (release date TBD). Thanks for bringing it to our attention.

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 11, 2016

Member

@jon-armstrong Thanks for updating us!

I plan to implement Edge's semantics of only ever assigning function declarations out of block via Annex B in Firefox soonish. Perhaps @littledan can weigh in on plans for V8?

Member

syg commented Feb 11, 2016

@jon-armstrong Thanks for updating us!

I plan to implement Edge's semantics of only ever assigning function declarations out of block via Annex B in Firefox soonish. Perhaps @littledan can weigh in on plans for V8?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Feb 12, 2016

Member

@syg want to try your hand at sending a PR to fix the spec?

Member

bterlson commented Feb 12, 2016

@syg want to try your hand at sending a PR to fix the spec?

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 12, 2016

Member

@bterlson Sure, I'll look at it early next week.

Member

syg commented Feb 12, 2016

@bterlson Sure, I'll look at it early next week.

@rgbkrk

This comment has been minimized.

Show comment
Hide comment
@rgbkrk

rgbkrk Feb 12, 2016

Thanks all and thanks for reaching out to us! 🎉 ES2015 🎉

rgbkrk commented Feb 12, 2016

Thanks all and thanks for reaching out to us! 🎉 ES2015 🎉

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Feb 12, 2016

Member

Thanks to @rgbkrk & @jon-armstrong providing data and helping us arrive at the correct path forward!

Member

bterlson commented Feb 12, 2016

Thanks to @rgbkrk & @jon-armstrong providing data and helping us arrive at the correct path forward!

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 12, 2016

Member

I'm not completely sure this particular is a big web compatibility issue, the way the duplicate-declarations-in-block one seems to be. Chrome has continued to not implement any sort of workaround, and we haven't gotten any bug reports even though it's already on Beta, and @jon-armstrong 's comments seem to indicate that they are changing their site. Is the PR supposed to be about this issue or the other one?

Member

littledan commented Feb 12, 2016

I'm not completely sure this particular is a big web compatibility issue, the way the duplicate-declarations-in-block one seems to be. Chrome has continued to not implement any sort of workaround, and we haven't gotten any bug reports even though it's already on Beta, and @jon-armstrong 's comments seem to indicate that they are changing their site. Is the PR supposed to be about this issue or the other one?

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 12, 2016

Member

@littledan The PR will be for this issue. While this particular issue with Rackspace is resolved upstream, I don't think there's any value in Annex B assigning anything other than function declarations out of the block as it currently does, since no implementation did that previously.

Member

syg commented Feb 12, 2016

@littledan The PR will be for this issue. While this particular issue with Rackspace is resolved upstream, I don't think there's any value in Annex B assigning anything other than function declarations out of the block as it currently does, since no implementation did that previously.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Apr 17, 2017

Member

We discussed this at the March 2016 TC39 meeting, and decided to go with the interpretation that we should allow this leaking, out of a sense of minimalism, even if it's counter-intuitive.

Member

littledan commented Apr 17, 2017

We discussed this at the March 2016 TC39 meeting, and decided to go with the interpretation that we should allow this leaking, out of a sense of minimalism, even if it's counter-intuitive.

@littledan littledan closed this Apr 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment