globalEval handling of "use strict" pragma incorrect.

[[email protected]…]

The function globalEval() assumes that if a "use strict" pragma is present in the code, it must be the first thing:

if ( code.indexOf("use strict") === 1 ) {
  script = document.createElement("script");
  script.text = code;
  document.head.appendChild( script ).parentNode.removeChild( script );
} else {
  // Otherwise, avoid the DOM node creation, insertion
  // and removal by using an indirect global eval
  indirect( code );
}

This is incorrect, as "use strict" must simply be the first statement, and may come after comments. I was unable to load the three.js library (​http://threejs.org/build/three.min.js) through ajax (jQuery.get, jQuery.getScript, jQuery.ajax) because of this.

I think possible solutions would be to strip comments (instead of just trimming whitespace) before doing the test, or to test that "use strict" comes before the first semicolon.

[dmethvin]

Seems like this just leads to a deeper rabbit hole. Trying to "parse" the script and ignore leading comments/spaces is going to be messy and time consuming.

[[email protected]…]

I think the time spent stripping leading comments would likely be less than that of inserting and removing unnecessary dom elements, but if it is too complicated, then it would probably be best to just always insert the script element, since it's the only way, without some preprocessing, to be sure the script will execute in the global context.

[jaubourg]

For ajax, you can just use crossDomaine: true in your option to force usage of the script transport.

As for a fix for this for globalEval, yes, using inline script tag injection is neat but there is no guarantee it'll execute synchronously. It failed in the past.

[[email protected]…]

I'm not sure I understand. If script tag injection is problematic, then why is it being used here for strict mode? What I'm suggesting is to always evaluate the code in the same way (injecting script tags) in both cases, and not have this broken test for strict mode. Or to fix the test for strict mode so that it actually works (stripping the comments, or otherwise properly checking for the directive).

Using eval on code in strict mode ​will not evaluate it in the global context, so this function simply does not work if there are any comments before the "use strict" directive in the code.

[dmethvin]

To me, any detection of "use strict" in *global* eval is incorrect since it should not be used in global scope. So the remaining question would be why we check for this at all. @jaubourg do you know?

[jaubourg]

After careful consideration, I think we should use script tag injection all the time. I remember asking rick about the two code paths back in the days and he answered something about performance concerns. Not sure the impact of injecting a script tag is that much of a concern here.

[dmethvin]

We're planning to use script tags always in 1.12/2.2, see #14757. I'll leave this one open separately.

[m_gol]

#14757 was fixed so there doesn't seem to be much more to do here. Closing.