Side navigation
#14189 closed bug (fixed)
Opened July 26, 2013 03:38PM UTC
Closed June 23, 2014 04:56PM UTC
globalEval handling of "use strict" pragma incorrect.
| Reported by: | tsherif@gmail.com | Owned by: | |
|---|---|---|---|
| Priority: | low | Milestone: | 1.12/2.2 |
| Component: | core | Version: | 2.0.3 |
| Keywords: | Cc: | jaubourg | |
| Blocked by: | Blocking: |
Description
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.
Attachments (0)
Change History (9)
Changed July 26, 2013 04:27PM UTC by comment:1
Changed July 26, 2013 05:21PM UTC by comment:2
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.
Changed July 26, 2013 08:57PM UTC by comment:3
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.
Changed August 01, 2013 11:22AM UTC by comment:4
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.
Changed August 20, 2013 06:16PM UTC by comment:5
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?
Changed October 18, 2013 02:40PM UTC by comment:6
| cc: | → jaubourg |
|---|---|
| component: | unfiled → core |
| priority: | undecided → low |
| status: | new → open |
Changed November 08, 2013 02:20PM UTC by comment:7
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.
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.