External scripts are evaluated async #331

brauliobo · 2013-11-06T18:41:42Z

As external scripts are removed from the content and then evaluated by inserting them into head, they end up being evaluated async which causes somes dependent symbols not to work. See this example

<script src='external_script_with_function'/>
<script type='text/javascript'>
  function_from_external_script_above();
</script>

function_from_external_script_above() won't work!

Commenting the line obj.scripts = findAll(obj.contents, 'script[src]').remove() workarounded the issue.

I understand that a sync evaluation may hang the application, but as this example demonstrate it is necessary on some cases. I suggest a configuration to support both behaviours or support only the sync behaviour.

The text was updated successfully, but these errors were encountered:

josh · 2013-11-06T18:46:29Z

Sorry, but there is no way to run "blocking" scripts as you can on an initial page load after the fact. Its just not possible with the way DOM fragment parsers work.

brauliobo · 2013-11-06T19:11:53Z

sorry @josh, can't understand your explanation.

what do you mean ' as you can on an initial page load after the fact'?
could you please elaborate more?

shogun70 · 2013-11-06T22:16:50Z

@brauliobo are you sure that commenting out that line made the scripts execute sequentially? I would expect this to not actually execute the scripts at all.

brauliobo · 2013-11-07T13:02:13Z

@shogun70 yes, I am, because this will make jquery's html() call to see those scripts

shogun70 · 2013-11-07T22:35:26Z

@brauliobo: Thanks - I didn't know html() executed scripts.

I think your suggestion makes more sense than the current behavior.
(But IMO anything that could be done with scripts in PJAX'd content would be better done a different way)

daGrevis · 2013-11-21T18:43:53Z

This “problem“ bothers me as well.

So I get a pjaxed request that contains HTML of the new page and some script[src] tags.

Scripts are not executed.

If I remove this from the source code:

    obj.scripts = findAll(obj.contents, 'script[src]').remove()
    obj.contents = obj.contents.not(obj.scripts)

...scripts are executed (well, alert(1) is executed correctly).

So what's the point of removing scripts from response?

I'm looking for a way to inject scripts when a page loads, don't want to load every script (script per page) in container.

Thanks!

daGrevis · 2013-11-21T19:06:22Z

To summarize, what's the point of removing scripts from HTML response?

lichunqiang · 2014-08-30T12:20:31Z

Wating the reason... :)

mislav · 2014-09-03T20:49:09Z

There are two types of <script> tags:

The ones with inline JavaScript in them. These are untouched by pjax but still get executed (eval'd) after the HTML inside the pjax container has been inserted via jQuery's .html() method.
The ones with src attribute (external scripts). We extract these from the pjax response and insert them into the <head> of the document after the new HTML has already been inserted into the pjax container. We only insert them in the document if there aren't already external scripts with the same src attribute, to prevent accidental repeated loading of the same external script.

If you think you've found a bug with this system please be more specific about what you're doing—which version of jquery-pjax are you using, what does your pjax response looks like, which script tag you expected to get executed but never did?

brauliobo · 2014-09-03T21:22:48Z

@mislav Please read the description of this issue. The problem is with the execution order, which is not preserved when using script and src.

mislav · 2014-09-03T21:52:04Z

Ah OK I understand. This is another example of how pjax doesn't (and can't) behave identically to static pages. You should construct your external script tags to not depend on each other at eval time, or handle script loading manually in your app using pjax events.

TL;DR; due to browser limitations, it's nontrivial for us to fix this without implementing a whole script loader withing pjax plugin.

brauliobo · 2014-09-03T22:26:38Z

I had proposed a way that is already handled by jquery automatically. jquery knows how to deal with script tags on the ajax response.

mislav · 2014-09-03T22:53:40Z

@brauliobo What does jquery do?

brauliobo · 2014-09-03T22:58:52Z

http://api.jquery.com/html/ and http://api.jquery.com/append/ handles script on the html they receive. Basically, it calls jQuery.globalEval() on inline scripts and jQuery.getScript() on external scripts (the ones with src attribute). I could not yet find documentation on this behaviour, I know it because I looked to jQuery's code.

That's why just commenting the line where pjax extract script tags fixed the problem, because later jQuery handled them.

ghost · 2014-12-09T16:14:04Z

The situation that @brauliobo (obrigado por apontar a linha culpada) describe is really common.
This issue has more than one year and the solution is really easy. No need to implement anything.
At least add something about this to the doc/wiki.

mislav · 2014-12-10T03:57:32Z

OK, let's clear something up first. Or, you people are welcome to prove me wrong.

Consider this HTML:

<p>Hello world!</p>
<script src="external1.js"></script>
<script src="external2.js"></script>
<script>External.something("foo")</script>

Here we have multiple external script loads, and an inline script that relies on the example External module loaded from those external scripts. Now, here's what I think I know about script loading:

When this HTML loads normally as part of a web page, each script[src] tag will block execution of everything after it, basically holding the page frozen until it finishes loading and executes the loaded script. Multiple external scripts are guaranteed to execute in DOM order. Then the inline script will run and everything will work.
When this HTML is instead inserted via jQuery's html(), jQuery will use getScript() to fetch and execute the script[src] contents. However, because this process is implemented with XHR and therefore is asynchronous, multiple scripts are not guaranteed to execute in DOM order, and they are not guaranteed to execute before the inline script (which jQuery evals). Since jQuery doesn't have any of this documented, this is just my guess. This is your chance to look into this and correct me.
When this HTML is instead inserted via pjax, we extract all script[src] and put them in HEAD so they automatically load and execute in async fashion. This is an alternative to dynamic script loading that we prefer because it doesn't rely on eval. However, the order of script execution is stil not guaranteed, and inline scripts are going to execute before the external scripts. This is an unfortunate side-effect and limitation of pjaxing your site, and is fixed by not having inline scripts that rely on external script execution blocking and order.

Now, nobody other than me would be happier if we managed to fix this and ensure that pjaxed sites work always exactly the same as native page loads. Because of that, I'm accepting constructive suggestions on how we can technically make this work. However, the solution can't depend on eval because we want pjax to be compatible with sites that use Content Security Policy to disable eval for safety reasons. One such site is GitHub.com, where we use pjax ourselves. Therefore, the solution can't rely on jQuery.getScript().

brauliobo · 2014-12-10T04:12:19Z

@mislav 2 is wrong. jQuery does guarantee execution order. I'm not sure, but I think it runs scripts syncronously when found by html()

mislav · 2014-12-10T04:17:11Z

@brauliobo OK, that might be true. Can you point us to somewhere where we can verify this? Perhaps a location in its source code

brauliobo · 2014-12-10T04:26:50Z

See _evalUrl and domManip from https://github.com/rails/jquery-rails/blob/master/vendor/assets/javascripts/jquery2.js#L5408 and https://github.com/rails/jquery-rails/blob/master/vendor/assets/javascripts/jquery2.js#L8316

brauliobo · 2014-12-10T04:27:20Z

In _evalUrl you see async: false

brauliobo · 2014-12-10T04:29:18Z

jQuery._evalUrl is a variable and can be replaced by the user.

mislav · 2014-12-10T04:31:45Z

Ah so it doesn't use getScript() exactly. Thanks for pointing that out.

OK so what we can do in pjax is try to detect whether eval is allowed in the current execution environment. If it's allowed, we can not extract script[src] and let jQuery do its job. If eval is not allowed by CSP, like on GitHub.com, we'll keep extracting all script[src] and put them into HEAD.

@josh How does that sound?

josh · 2014-12-10T05:13:18Z

Its not really possible in CSP 1.0 to feature detect if eval() is allowed in a sane way.

mislav · 2014-12-10T05:51:32Z

No? What happens if you call eval("1+2")

josh · 2014-12-10T06:09:20Z

It will crash and send back a CSP violation ping back to our servers each
time.
On Tue, Dec 9, 2014 at 9:51 PM Mislav Marohnić notifications@github.com
wrote:

No? What happens if you call eval("1+2")

—
Reply to this email directly or view it on GitHub
#331 (comment).

brauliobo · 2014-12-10T10:39:27Z

I haven't able to fully test this but it seems promising:

function cspEnabled() {
  try {
    eval("return false;");
  } catch (e) {
    return true;
  }
}

from http://stackoverflow.com/questions/19140169/how-to-detect-content-security-policy-csp

brauliobo · 2014-12-10T10:51:43Z

it is pretty similiar to what angular.js is doing: IgorMinar/angular.js@e05b78a

angular/angular.js#8162

josh · 2014-12-10T16:47:08Z

Again, that causes a violation report which is unacceptable.
On Wed, Dec 10, 2014 at 3:03 AM Leonardo Tietböhl notifications@github.com
wrote:

@josh https://github.com/josh from CSP 1.0

If 'unsafe-eval' is not in allowed script sources:

Instead of evaluating their arguments, both operator eval and
function eval must throw a security exception. [ECMA-262]

—
Reply to this email directly or view it on GitHub
External scripts are evaluated async #331 (comment).

brauliobo · 2014-12-10T18:06:52Z

@josh So a CSP disable/enable option is the only alternative. (just like angular.js did)

josh · 2014-12-10T18:38:44Z

I don't really like the general idea of having two different modes that
behave pretty differently. Not appending to head means that pjax can't
track scripts it's eval'd in a consistent way.
On Wed, Dec 10, 2014 at 10:07 AM Bráulio Bhavamitra <
notifications@github.com> wrote:

@josh https://github.com/josh So an CSP disable/enable option is the
only alternative. (just like angular.js did)

—
Reply to this email directly or view it on GitHub
#331 (comment).

brauliobo · 2014-12-10T22:42:26Z

@josh Tracking scripts should be as easy as storing the list of src in a variable on pjax's context.

mislav · 2014-12-11T00:11:59Z

On Wed, Dec 10, 2014 at 10:06 AM, Bráulio Bhavamitra <
notifications@github.com> wrote:

So an CSP disable/enable option is the only alternative.

It seems so, which sucks a lot because I don't want that to be an option.
Developers of web apps would have to remember to change pjax configuration
when transitioning to CSP, and we would have to maintain separate code
paths.

I'd rather just draw a line and say we as a library won't support sites
that rely on inline scripts and blocking, ordered external scripts load
from pjax-loaded page bodies.

nkovacs · 2015-05-04T19:24:46Z

This commit uses onload to load the scripts serially, in the original order (excluding already loaded scripts). It also loads inline scripts in the original order, by appending them to the container (so it only relies on eval if you have inline scripts, like pjax does now).
You do lose parallel script downloads, but this is necessary for correctness. It's still async though. I've only tested this on Chrome, and I forked Yii's version of pjax, because I needed it for Yii 2.

mislav · 2015-05-12T12:12:34Z

This commit uses onload to load the scripts serially, in the original order (excluding already loaded scripts). It also loads inline scripts in the original order, by appending them to the container (so it only relies on eval if you have inline scripts, like pjax does now).

I looked at the implementation and it seems solid. We might look into including something like that if we can simplify the implementation, and if the <script> tag onload events are reliable cross-browser. However right now I would much rather introduce a configuration option that lets you switch between jQuery mode (sequential eval) and current async mode (modern, CSP-enabled websites).

brauliobo · 2015-05-12T12:18:49Z

@mislav +1

dmill-bz · 2015-10-07T10:31:58Z

@mislav +1 on at least having a configuration option for this. Has there been any progress with this since?

mislav · 2015-10-07T10:38:40Z

No progress since. I wasn't super motivated to prioritize this since pjax works very well for our use case and we generally don't need loading in sync.

arobbins · 2016-08-06T17:41:00Z

Are there still plans to follow through with this feature?

staabm · 2016-08-06T18:24:26Z

Couldnt code like quoted in the inital report use the onload event to intialise properly?

<script src='external_script_with_function.js' onload='function_from_external_script();'/>

nkovacs · 2016-08-06T18:33:06Z

That's not a solution if you have an existing site you'd like to pjaxify and especially if the code is out of your control, because it's part of a framework, like Yii 2.

RichardTMiles · 2017-07-29T21:50:03Z

The filament group has a repository called loadJS : https://github.com/filamentgroup/loadJS/blob/master/loadJS.js

<script>
/*! loadJS: load a JS file asynchronously. [c]2014 @scottjehl, Filament Group, Inc. (Based on http://goo.gl/REQGQ by Paul Irish). Licensed MIT */
(function( w ){
	var loadJS = function( src, cb ){
		"use strict";
		var ref = w.document.getElementsByTagName( "script" )[ 0 ];
		var script = w.document.createElement( "script" );
		script.src = src;
		script.async = true;
		ref.parentNode.insertBefore( script, ref );
		if (cb && typeof(cb) === "function") {
			script.onload = cb;
		}
		return script;
	};
	// commonjs
	if( typeof module !== "undefined" ){
		module.exports = loadJS;
	}
	else {
		w.loadJS = loadJS;
	}
}( typeof global !== "undefined" ? global : this ));

 $(document).on('pjax:end', function () {
             $(document).on('submit', 'form[data-pjax]', function (event) {
                        $.pjax.submit(event, '#ajax-content')
             });
              loadJS("components/jquery/jquery.min.js' ) ?>", function () {
                      loadJS("bower_components/bootstrap-datepicker/js/bootstrap-datepicker.js", function (){
                             $('#datepicker').datepicker({autoclose: true});
                    });
            });
 });
</script>

Is this a possible solution?

…for surveymenus

josh closed this as completed Nov 6, 2013

rounce mentioned this issue Oct 19, 2014

replace content after scripts have been loaded #446

Closed

mislav reopened this Dec 10, 2014

ghost mentioned this issue Dec 26, 2014

Pjax and inline scripts yiisoft/yii2#6655

Closed

dallaslu mentioned this issue Feb 4, 2015

remove calling of the idiot executeScriptTags #479

Closed

mislav mentioned this issue Feb 4, 2015

that scripts are moved to head, will lead to a fetal erorr with inline script depends on the script files. #478

Closed

mislav mentioned this issue Mar 3, 2015

Add native support beforeSend and fix for asynchronous requests #423

Closed

mislav mentioned this issue Mar 13, 2015

Recent commit causes issue with external and inline scripts #502

Closed

mislav added the bug label Mar 13, 2015

mislav mentioned this issue Mar 30, 2015

Pjax + inline script + ajax #510

Closed

dmill-bz mentioned this issue Apr 2, 2015

Problem with inline scripts and Pjax yiisoft/yii2#4814

Closed

larissa mentioned this issue Apr 17, 2015

Plugin Sniffer: Map Asynchronous Loading CIRANDAS/noosfero-ecosol#41

Merged

Thibaut mentioned this issue Apr 24, 2015

Scripts running out of order turbolinks/turbolinks-classic#513

Closed

This was referenced Oct 18, 2015

Inline script need preload js file. #587

Closed

fixed jquery.ready before script file loaded #588

Closed

nkostadinov mentioned this issue Mar 4, 2016

Issue with external script and PJAX yiisoft/jquery-pjax#41

Closed

fari-99 mentioned this issue Apr 22, 2016

reload javascript external source(?) #624

Closed

reefdog mentioned this issue Oct 18, 2016

Asynchronously load note embeds documentcloud/documentcloud-notes#11

Merged

Shnoulle referenced this issue in LimeSurvey/LimeSurvey Sep 12, 2017

Dev: fixed a lot of small itches and added an active/inactive switch …

9c3f07f

…for surveymenus

External scripts are evaluated async #331

External scripts are evaluated async #331

Comments

brauliobo commented Nov 6, 2013

josh commented Nov 6, 2013

brauliobo commented Nov 6, 2013

shogun70 commented Nov 6, 2013

brauliobo commented Nov 7, 2013

shogun70 commented Nov 7, 2013

daGrevis commented Nov 21, 2013

daGrevis commented Nov 21, 2013

lichunqiang commented Aug 30, 2014

mislav commented Sep 3, 2014

brauliobo commented Sep 3, 2014

mislav commented Sep 3, 2014

brauliobo commented Sep 3, 2014

mislav commented Sep 3, 2014

brauliobo commented Sep 3, 2014

ghost commented Dec 9, 2014

mislav commented Dec 10, 2014

brauliobo commented Dec 10, 2014

mislav commented Dec 10, 2014

brauliobo commented Dec 10, 2014

brauliobo commented Dec 10, 2014

brauliobo commented Dec 10, 2014

mislav commented Dec 10, 2014

josh commented Dec 10, 2014

mislav commented Dec 10, 2014

josh commented Dec 10, 2014

brauliobo commented Dec 10, 2014

brauliobo commented Dec 10, 2014

josh commented Dec 10, 2014

brauliobo commented Dec 10, 2014

josh commented Dec 10, 2014

brauliobo commented Dec 10, 2014

mislav commented Dec 11, 2014

nkovacs commented May 4, 2015

mislav commented May 12, 2015

brauliobo commented May 12, 2015

dmill-bz commented Oct 7, 2015

mislav commented Oct 7, 2015

arobbins commented Aug 6, 2016

staabm commented Aug 6, 2016 • edited Loading

nkovacs commented Aug 6, 2016

RichardTMiles commented Jul 29, 2017 • edited Loading

staabm commented Aug 6, 2016 •

edited

Loading

RichardTMiles commented Jul 29, 2017 •

edited

Loading