JavaScript Patch #4: External Scripts

this is required for some much more modules, too. for e.g. Google Analytics.

This patch looks like RTBC...

Additional something i found - it seems not possibly to add JS inline code in the closure variable. This is for e.g. required for Google Analytics and some other statistic modules. This seems currently not possible or i was only unsuccessful... 'header' and 'footer' works but 'closure' not.

It would certainly make referencing remote javascript urls cleaner inside drupal modules. So the feature request gets a +1 for me to use in the Google Analytics module.

There was a bug inside the patch. I fixed the ordering of JS files embedded into HTML and tested successful with Google Analytics under D51. Nevertheless the patch is build against today HEAD.

This is the example code for current Google Analytics module.

drupal_add_js('http' . $prefix . '.google-analytics.com/urchin.js', 'external', 'footer');
drupal_add_js("<!--\n_uacct = \"".$id."\";{$segmentation}{$codesnippet}urchinTracker();\n// -->\n", 'inline', 'footer');

With the patch provided in #2 the order of JS ends with the following wrong code order:

<script type="text/javascript"><!--
_uacct = "UA-736352-1";__utmSetVar('');urchinTracker();
// -->
</script>
<script type="text/javascript" src="http://www.google-analytics.com/urchin.js"></script>

This is now fixed with this patch and produces the following output. This loads first the external files with the functions possibly required by the types "inline" or "settings":

here is the missing part of my last posting:

<script type="text/javascript" src="http://www.google-analytics.com/urchin.js"></script>
<script type="text/javascript"><!--
_uacct = "UA-736352-1";__utmSetVar('');urchinTracker();
// -->
</script>

This looks good to me.

Can't you work around this with drupal_set_html_head()? If so, I'd like to not commit this to 5.x, as it's an API change and 5.x modules that use it will break without it.

This was original build as a feature for D6 for not using a workaround like drupal_set_html_head()... if you'd like to commit to D5, well - we are happy, too. drupal_set_html_head() may work.

case 'external': could be placed after case 'inline':? Because it would be the rarest option used in drupal_add_js and this is just a tiny performance tweak :) Changing status..

Sorry, but you are wrong. Read my above posting, please. we first need to add JS code - no matter if - internal or external and then we must add inline code or settings code. if we do different, we cannot call a function from inline code that is inside the included JS files. therefor this order is a *must*.

Well, if you think so, you are incorrect. With this order you can't accomplish what you are trying to explain. You are not passing all the JS and options altogether to drupal_add_js where it decides which JS code to output first and what second. This order is just normal syntax.

if my code looks like

<script type="text/javascript"><!--
_uacct = "UA-736352-1";__utmSetVar('');urchinTracker();
// -->
</script>

<script type="text/javascript" src="http://www.google-analytics.com/urchin.js"></script>

i get a function urchinTracker() is not defined error.

If i use my patch the order is done in the following way and i have NO function undefined error.

<script type="text/javascript" src="http://www.google-analytics.com/urchin.js"></script>

<script type="text/javascript"><!--
_uacct = "UA-736352-1";__utmSetVar('');urchinTracker();
// -->
</script>

The order is therefor very important - if you have functions that are executed one page load, isn't it?

Yes it's important, and to accomplish that, you really don't need to patch drupal. Just use it like:

drupal_add_js(..., 'external');
drupal_add_js(..., 'inline');

So, your point is not related here. :) And the patch seems to be fine too. :)

Yes it's important, and to accomplish that, you really don't need to patch drupal. Just use it like:

drupal_add_js(..., 'external');
drupal_add_js(..., 'inline');

So, your point is not related here. :) And the patch seems to be fine too. :)

Sorry i don't understand you. There is currently no drupal_add_js(..., 'external'); in drupal core. So we need to patch drupal. why are you writing we don't need to patch? We need to patch.

If you think the patch is fine set to RTBC, please. So we have a chance to get this into D6 core. D6 code freeze is only 2 weeks away... THX

I mean you dont have to patch anything to get the series of external or inline, up or down. Sure you have to patch to get the external option in. Hope you get it. Don't worry, whatever has to, will get in on time.

I created this workaround until the external is added to drupal_add_js.

I needed to add Amazon affiliate script at the end of my document similar to the earlier post that needed a Google Analytics post. I did it by creating a text file called affiliate.js. All it contained was:

Then I used a php include to include it at the bottom of my post:

include('sites/js/affiliate.js');

You can see what it does at: http://www.bradrice.com/drupal/node/8

I would still like to see the code updated to allow for external links, but this is a temporary workaround.

Here is a slightly extended patch which
1. adds documentation for the 'external' option to drupal_add_js()
2. fixes a small coding-style issue (was present before the patch)
3. allows the external files to be preprocessed optionally

I really hope we can get support for 'external' javascript into D6. Its a much needed feature IMO, esp. for integration modules.

@Steven's question 'Can't you work around this with drupal_set_html_head()?'

Here is why I think this is not a satisfying solution ...
- drupal_set_html_head does not check for duplicated includes (drupal_add_js does)
- drupal_set_html_head doesnt cascade with other js includes. It always adds the files at the beginning of the head section, even before jquery.js, etc. Thats especially bad if you have an external application to embed, which also depends on jquery. You must include jquery manually in this case and it will be added twice in the end.

@profix898: are you sure preprocess will work with this patch? I don't think so... maybe i missed something, but you need to send a HTTP request to an external box, save the content to disk and so on and so on... I think this is no good idea to integrate external scripts into your local scripts... this will break many external scripts, too. And finally if the remote site change the file your local cache is not updated and therefor may not work until regenerated.

calling external HTTP request is problematic... if the remote server is down your server may goes down while it starts with tons of hanging HTTP request to the JS file on the external site.

@hass: drupal_build_js_cache() simply uses $contents .= _drupal_compress_js(file_get_contents($path). ';'); to read the file and AFAIK file_get_contents() also works for remote files. I think it should work that way, yes. I also dont think this would break the external script. If the script changes frequently you cant still call drupal_add_js() with preprocessing disabled and the cache is not updated for local modified files either.

BUT I agree about the HTTP request being problematic. If the external source is unavailable the server will hang instead of the browser. So lets remove that option. Attached patch is quite similar to yours but fixes the coding-style issue and adds docs.

Why does the 'external' option allow an array to be passed in, when this is not done for local files? (at least according to the doxygen). This would be inconsistent.

As for the drupal_set_html_head() workaround, please read my comment carefully. The original post asks whether this a bug report or a feature request. My point was that since the bug fix requires an API change, it was unlikely to be backported to 5.x, but if there was a workaround available, that there would no real reason to do so.

@Stephen: as stated above - we are not talking about a Bugfix. This is a Feature Request for D6, nothing else.

Sorry, I was wrong :( The 'external' option does - of course - not allow an array to be passed. Changed the doxygen accordingly ...

I still think this is an important feature and should go in ...

For Google Analytics feature request in http://drupal.org/node/182334 we need this fixed and i'm bumping this up now to get into D6.

Updated patch for latest HEAD. No changes - only updated to remove hunks.

Simple decision

drupal_add_js('</script><script type="text/javascript" src="http://maps.google.com/maps?file=api&amp;v=2&amp;key=ABCDEFG" >', 'inline');

Subscribing...

Updated to HEAD and tested it by using the following contents inside a new node with the PHP Filter:

  // Add an external Javascript file
  drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE', 'external');

  // Add the Javascript to test it
  drupal_add_js('window.onload = function() {
      if (GBrowserIsCompatible()) {
        var map = new GMap2(document.getElementById("map_canvas"));
        map.setCenter(new GLatLng(37.4419, -122.1419), 13);
      }
  }', 'inline');

  echo '<div id="map_canvas" style="width:500px; height:300px;"></div>';

It displays a Google Map by using an external Javascript file from Google. You'll need a Google Maps API key generated here.

API changes are not accepted in Drupal 6, unless they fix bugs. This adds a new feature, so postponed. Just as it was with 5.x (see above), this came too late. Please make sure you get this into 7.x early.

Robby Rob If its not too much to ask, could you update the patch for the first RC? Too bad this one didn't make 6!

Subscribing. Why is it so hard to add such a seemingly simple feature? Don't get why this wasn't committed ages ago.

This patch will have to be moved to HEAD. It would also be great if somehow we got caching working on the external scripts.

Please make caching for external an extra patch. this one is RTBC

Discussion of adding caching for the external Javascript files can continue here: Cache External Javascript.

Patch is getting hunk failures.

Rerolled to HEAD...

To test:

1. Fresh Drupal install
2. Apply the patch
3. Enable the PHP input filter module at admin/build/modules
4. Create a new page at node/add/page using the new PHP Input Filter with the following code:
   

     // Add an external Javascript file
     drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE', 'external');
   
     // Add the Javascript to test it
     drupal_add_js('$(document).ready(function() {
         if (GBrowserIsCompatible()) {
           var map = new GMap2(document.getElementById("map_canvas"));
           map.setCenter(new GLatLng(37.4419, -122.1419), 13);
         }
     });', 'inline');
   
     echo '<div id="map_canvas" style="width:500px; height:300px;"></div>';
   

5. Save the page and view it
6. You should see a Google Map

As you can see, in this code, we call drupal_add_js using the external flag, meaning it will load the Google Maps API externally without having to hold it on the local server.

@hass: drupal_build_js_cache() simply uses $contents .= _drupal_compress_js(file_get_contents($path). ';'); to read the file and AFAIK file_get_contents() also works for remote files.

No it doesn't. This is why Drupal implements its own HTTP client function with drupal_http_request and uses only this to get remote files. file_get_contents can be enabled for URLs, but most hosts shut this off for security, to ensure that you only access a URL when you really mean to.

I know the patch no longer supports preprocessing for remote files, but this is an important aspect of retrieving external data anywhere in core, so it bears repeating.

Or try this instead :

  $path = drupal_get_path('module', 'mymodule');
  drupal_add_js($path .'/downloadscript.js');

then in downloadscript.js:

 var source= ('http://www.google.com/uds/api?file=uds.js&v=1.0&key=MYAPIKEY');
 document.write ("<scr"+"ipt type='text/javascript' src='"+source);
 document.write ("'><\/scr"+"ipt>");

That work-around is a horrible hack, and still results in the same issue at hand. Although it successfully allows us to use external scripts though drupal_add_js, it doesn't do it cleanly at all. We should be able to put the script URL right into the method call.

I think caching and preprocessing these external scripts should be handled in a seperate issue than what this patch is doing. The cache would download the referenced script to the files directory, and then reference the JS file in the files directory. This seems like a lot of work for one patch, so splitting it into two will help the situation. Could we get a review of this patch?

#39 works for me. The following test code was used in a PHP-evaluated node:

<?php drupal_add_js('http://ermarian.net/style/jquery.form', 'external'); ?>
<form id="testform">
  <input type="submit" value="Start" />
</form>

<script type="text/javascript">
  $(document).ready(function() {
    $('#testform').ajaxForm({beforeSubmit: function() {alert("Forms extension present.");}});
  });
</script>

The jQuery forms extension is not in Drupal, so pre-patch (with the URL included as a relative path) the missing library will cause an error. Post-patch, clicking on the button will pop up the alert attached to it via ajaxForm.

Subscribing.

jQuery forms extension is not in Drupal

Whoops, correction: The jQuery forms extension is only loaded when a form requiring AHAH behavior is built. The test code above will not work if an AHAH form is present on the page - it will work without the patch just as it will with the patch.

So, has anyone reviewed this patch successfully? If so, can we get a RTBC?

I've tested it. Perhaps we can get a second review?

Updated to HEAD. Test still applies:

1. Fresh Drupal HEAD install
2. Apply the patch
3. Enable the PHP input filter module at admin/build/modules
4. Create a new page at node/add/page using the new PHP Input Filter with the following code:
   

     // Add an external Javascript file
     drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE', 'external');
   
     // Add the Javascript to test it
     drupal_add_js('$(document).ready(function() {
         if (GBrowserIsCompatible()) {
           var map = new GMap2(document.getElementById("map_canvas"));
           map.setCenter(new GLatLng(37.4419, -122.1419), 13);
         }
     });', 'inline');
   
     echo '<div id="map_canvas" style="width:500px; height:300px;"></div>';
   

5. Save the page and view it
6. You should see a Google Map

The attached patch also caches the external Javascript files locally. Reviews would be great.

@Rob: caching of external files should go into an extra patch http://drupal.org/node/91250#comment-722222. See your own comment below this comment.

Then this patch should suffice for this issue...

obligatory sub comment

Similar thread can be found here: http://drupal.org/node/251578

Rob et al, it would be great if you could share your thoughts on #251578 at http://drupal.org/node/251578. It seems to fix this order, as well fix a problem with the order in which Javascript files are shown. We might conclude that this patch is a duplicate ... ?

Interesting discussion but everybody forget about collisions of variable names and security.

I ported the code over from the $type = 'external' code to just check if the path given is a valid absolute URL in the reinvention of drupal_add_js.

This becomes patch #4 in this list for #251578: More flexible js/css ordering and an alter operation.

#315798: JavaScript Patch #2: Weight brings up some refactoring of the $javascript array that have to go in before we can look at this.

This patch adds an 'external' type for drupal_add_js(), as well as a couple tests.

Sticking the following in a node with the PHP filter works.......

  // Add an external Javascript file
  drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE', 'external');

  // Add the Javascript to test it
  drupal_add_js('$(document).ready(function() {
      if (GBrowserIsCompatible()) {
        var map = new GMap2(document.getElementById("map_canvas"));
        map.setCenter(new GLatLng(37.4419, -122.1419), 13);
      }
  });', 'inline');

  echo '<div id="map_canvas" style="width:500px; height:300px;"></div>';

Any thoughts on sticking the 'external' functionality in the 'file' type, with a check to see if we're using an absolute URL?

We do the "sticking the 'external' functionality in the 'file' type, with a check to see if we're using an absolute URL" already on http://api.drupal.org/api/function/l/7 function, no?

url has it...

$colonpos = strpos($path, ':');
$options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
 

Apparently that's the check to see if a string is an absolute URL. Could we use that to replace 'external' with 'file', and just allow people to send absolute URLs to external scripts using the 'file' type?

I like that idea. One fewer parameter for developers to remember.

Also, $type's PHPDoc could use specifying which option there is the default if unspecified.

I forgot about valid_url. This patch allows the use of external JavaScript files via the JavaScript 'file' type.... The following in a node works:

  // Add an external Javascript file
  drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE');

  // Add the Javascript to test it
  drupal_add_js('$(document).ready(function() {
      if (GBrowserIsCompatible()) {
        var map = new GMap2(document.getElementById("map_canvas"));
        map.setCenter(new GLatLng(37.4419, -122.1419), 13);
      }
  });', 'inline');

  echo '<div id="map_canvas" style="width:500px; height:300px;"></div>';

I'm new here, and not quite sure what are trying to achieve, but I worked around a similar problem by moving the code I wanted to call out into a text file (eg. googlecse.html). Then I created the block, and added the PHP code:

<?php
include_once("googlecse.html");
?>

cheers,
David.

Test failure : #335122: Test clean HEAD after every commit

A couple thoughts...

Should we allow external files to be pulled into our preprocessed aggregate files? I think of this in the spirit of having less js files included in the page.

Should we provide a method for adding files before the cached file? Think of the use case where you include prototype via http://ajax.googleapis.com/ajax/libs/prototype/1.6.0.3/prototype.js and then want to use it in your local files. Those files cannot be aggregated unless prototype.js is included before the aggregate file.

Also, I would swap the if/else statement to read:

// External scripts are embedded straight on the page.
if (!valid_url($item['data'], TRUE)) {
  $files[$item['data']] = $item;
  if (!$item['preprocess'] || !$is_writable || !$preprocess_js) {
    $no_preprocess .= '<script type="text/javascript"' . ($item['defer'] ? ' defer="defer"' : '') . ' src="' . base_path() . $item['data'] . ($item['cache'] ? $query_string : '?' . REQUEST_TIME) . "\"></script>\n";
  }
  else {
    $files[$item['data']] = $item;
  }
}
else {
  $no_preprocess .= '<script type="text/javascript"' . ($item['defer'] ? ' defer="defer"' : '') . ' src="' . $item['data'] . "\"></script>\n";
}

This would put the if clause being used more than the else clause.

Are we looking to make sure the URL is valid or that the URL is actually a URL. If we are just looking for a url (that could potentially be invalid) we might consider doing something like:

  filter_var($item['data'], FILTER_VALIDATE_URL, FILTER_FLAG_SCHEME_REQUIRED)

instead of:

  valid_url($item['data'], TRUE);

filter_var has some limitations (see http://www.talkincode.com/php-filter-filter_validate_url-limitations-124...). But, it will tell us it it's an attempt at a url and should be faster than a regex (though I would test that). Aren't we really trying to see it it's an attempt at a url and not just a valid one? If someone enters an invalid url we don't want to try to include that as a local file, do we?

For some details on using filter_var this way see http://www.w3schools.com/PHP/filter_validate_url.asp

The patch at #49 cached the file locally, but I'm not too sure how I feel about that yet. What if the external script is expected to change? What if the request to the external file fails? Could we run into legality issues if we host the file? We could cache it, just not too sure if it's a good idea yet ;-)..... Thoughts?

Thanks for bringing up filter_var! That regex in valid_url looked pretty ugly.

It turns out this is a much more complicated issue than I imagined. For example, if someone users hook_js_alter on the jquery path to change it to http://ajax.googleapis.com/ajax/libs/jquery/1.2.6/jquery.js and preprocessing was turned on all the core scripts would break. jQuery would end up included as a non-preprocessed file after the preprocessed ones. This just doesn't work.

So, how do we handle this in light of our preprocessing and object dependency?

An option is to require jquery.js be loaded first and use something like http://docs.jquery.com/Ajax/jQuery.getScript

Thoughts? Other options? Do we just throw dependencies out the window here and let module authors deal with it? Do we have this a pluggable system so module authors can change it?

An idea.....

First we create a new js file (something like drupaljsloader.js) with a function in there DrupalJS.jsload (or something like that). The reason for the new namespace is because it could be used in cases where drupal.js (and thus the Drupal namespace) is not loaded, yet.

If we have an external file in the preprocessing we add drupaljsloader.js to the preprocessed file first. Then, DrupalJS.loader(http://example.com/filename.js) is called which loads the external file. We use this instead of jQuery.getScript so it removes the dependency on jQuery (which means we can use this on jQuery itself). jQuery.getScript uses jQuery.ajax() which has a lot more overhead. We could implement a similar concept and strip out a lot of the logic that we don't need.

This would allow us to keep our object/js file dependency (I think).

Thoughts?

Before I go down a road or writing this I would appreciate hearing back from a core maintainer or a js guru.

I think we should recreate as little code as possible here, so using jQuery.getScript seems very reasonable. This patch:

• Uses jQuery.getScript when aggregating external files
• Adds a test that adds an external file and sees if it was added correctly
• Uses filter_var to see if its an external file
• Tests pass, and the manual test at #64 works when aggregation is either on or off

This looks like a good start. Though, $.getScript should be jQuery.getScript.

The problem with getScript() is it will only work for external scripts weighted after jquery.js. If we want to replace jquery.js with one from google this will fail. getScript, also, fires off a number of other ajax functions to jquery plugins we may not want fired. We are looking for a simple loader.

I have something in mind... working on a patch.

+          $no_preprocess .= '<script type="text/javascript"' . ($item['defer'] ? ' defer="defer"' : '') . ' src="' . $prefix . $item['data'] . ($item['cache'] ? $query_string : '?' . REQUEST_TIME) . "\"></script>\n";

I think you should call check_plain() on $item['data'] (just like url() does) so that users don't have to HTML-encode the URL themselves, i.e. so that they can write
drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE');
instead of
drupal_add_js('http://maps.google.com/maps?file=api&v=2&key=GOOGLEKEYHERE');

Also, if you append $query_string or '?'.REQUEST_TIME to external URLs, you should use & instead of ? if the URL already contains a ?. I guess it is safer to not add anything to external URLs - some servers may not like additional query string parameters.

In the test you may want to link to an example.org-URL rather than an actual URL outside the control of Drupal (the JS file is never fetched, so it doesn't matter that it is not a real URL).

#73, though a valid point, seems a bit of an edge case to me. i agree with rob that we should just document that. if someone's trying to override jquery.js, they hopefully know enough of what they're doing to know to read the documentation and figure out their own work-around.

@aaron replacing jquery.js is first and most common use case I've herd.

Plus, I don't think it would actually work with getScript(). The call to getScript would fire off to get a js file. While it's still receiving that file the rest of the js in the preprocessed file will be executed. If anything else depends on it we have a race condition. Did the file load or not at this point?

I've been hacking around with this and am not happy with anything I've written as being core quality. The solution I've got works in a similar way to getScript() but isn't dependent on jQuery and removes the race condition. But, it isn't pretty (by my standards).

I'm starting to lean on the preprocess the contents of everything into one file. Then we provide a pluggable setup for preprocessing and make it easy for anyone to override. Then, I'll provide my other way as an alternative.

I hope to post some code in the next week. I want to play with it some more and see if I can make it nicer.

The attached patch enables external JavaScript files to be included. The external files are preprocessed/cached locally just like the others in drupal where they all are pulled into to the preprocessed files.

External files need to fit in the flow of dependencies in many cases so they can't just be included after the preprocessed file. For example, if we grabbed jQuery or jQuery UI from google.

It would be great to link to these files and pause the execution of the preprocessed file while the external script loads. I have not been able to make that work due to the way browsers parse through the js on a page. For example, if I replace jquery.js with one from google the jQuery.extend for Drupal.settings in the page executes before the external script is loaded and drupal.js can execute. This causes an error in the page.

I imagine we can come up with a better way(s) to preprocess and cache the files. But, that shouldn't hold up this patch. We can either file another patch later or create a contrib module if #352951: Make JS & CSS Preprocessing Pluggable gets into core.

FYI, this patch is basically what Rob Loach had been doing in earlier patches.

Seems there was an issue on a testing server. Applies fine. Setting back to CNR.

Discussed in IRC with dmitrig01. We should set external url preprocess default to FALSE.

Is this an old patch? I don't see this working when JavaScript Optimization is on.

The attached patch adds that the default preprocessing for external files is set to false. I'm not sure I like how I did this. Other than this change it's the same as the patch in #77.

@Rob Loach - when JS optimization is on the external files can be rolled into the cached file in this patch (they were rolled in by default in the previous patch). To reproduce go to an add node page, set the body input format to PHP, and insert this code into the body

$external = 'http://yui.yahooapis.com/2.6.0/build/utilities/utilities.js';
drupal_add_js($external, array('preprocess' => TRUE));

Preview the page, and take a look in the preprocessed js file for the page. You'll find the YUI utilities.js file included.

This is probably the best we can do without hitting jQuery.getScript when preprocessing is on. What did you have going before?

@Rob Loach - I was working on a lightweight version of $.getScript() that worked outside of jQuery and worked in our setup. $.getScript() has a bit of overhead because it really just uses $.ajax(). I wasn't able to get my custom method to work due to race conditions. For the same reasons $.getScript() doesn't work either.

JS execution order is the issue. For example:

$.getScript('somescript.js);

$('#someelement').methodFromsomescript();

The method methodFromsomescript() will execute before getScript has finished downloading the script. Their solution is to do something like:

$.getScript('somescript.js, 'mycallback');

var mycallback = function() {
  $('#someelement').methodFromsomescript();
}

This, too, can run into race conditions. For example, make somescript drupal.js and then you'll end up with an error from jQuery.extend for Drupal.settings in the page because drupal.js isn't loaded yet and the Drupal object doesn't exist.

I didn't want this to old up this patch anymore. We might not be able to solve this easily (if at all).

If #352951: Make JS & CSS Preprocessing Pluggable goes in we can work on a different method in contrib.

Agreed, I think this is the best we can get without caching the file locally on the server.

Just got an idea, contrib modules could change the desired effects of "external" JavaScript if we make a "default" use case for the switch checking the type. If we change the 'case "inline":' type to 'default:', a contrib module could essentially add any type of JavaScript they wanted. hook_js_alter would let contrib modules modify its behavior. Even if we don't get external JavaScript into core, it could become a contrib module. Taking this into consideration, I think we should make external JavaScript strictly referenced via "external" instead of "file". hook_js_alter would really benefit from it (would save developers from having to go through checking to see if the file is external or not).

Somewhat unrelated, but the google.load function is pretty awesome and demonstrates why allowing external JavaScript is a definite good thing.

Here's my proposed plan based on what Rob Loach wrote:

• We change file: to default: on the switch statement for handling teh generation of js links.
• We detect if a file is external and change the type to external. This will make js_alter calls easier for external files.
• We default external files to have preprocess set to false.

This along with #352951: Make JS & CSS Preprocessing Pluggable would mean a contrib module could implement something like jsload or google.load for the libraries it handles.

This doesn't provide anything extensive but it makes future expansion possible. Thoughts?

I find it illogical that such a simple and essential tool was not programmed in to the earliest versions of Drupal. As for all these patches, I'm just going to skip that and program the thing right in to my page.tpl.php file. Not even a realistic tutorial for newcomers... I've been able to do most other things in Drupal but this is where I've wasted a lot of time.

@roboc0p: the only way these simple and essential tools get programmed into Drupal is if people work on them. It sounds like this is an "itch" of yours. It at the very least caused you to lose a bunch of time. Wouldn't it be wonderful to save the next person from that (who might actually be you 9 months down the road after you forget how it was done?)

It would be great if we had you on the team of people working on Drupal core JS issues. Check out http://drupal.org/community-initiatives/drupal-core for some basic instructions and http://drupal.org/node/362430 for a list of JS improvements, specifically. If you need a hand getting a patch testing/creating environment set up, drop by #drupal on IRC and someone would be happy to help. :)

This patch takes #84 by mfer into consideration. It explicitly asks for 'external' or 'file' so that we don't have to check through filter_var every time a file is added. It doesn't cache the file locally, but if that's something we want, it's definitely something that could be handled through hook_js_alter through contrib, or a later patch.

After applying this patch, sticking the following in a node with the PHP filter will use google.load to load SWFObject, and then embed a YouTube document on the page.

  // Add an external Javascript file (jmaps)
  drupal_add_js('http://www.google.com/jsapi', 'external');

  // Add the Javascript to test it
  drupal_add_js('
  google.load("swfobject", "2.1");
  google.setOnLoadCallback(function() {
     swfobject.embedSWF("http://www.youtube.com/swf/l.swf?swf=http%3A//s.ytimg.com/yt/swf/cps-vfl76849.swf&video_id=5uZr3JWYdy8&rel=0&eurl=http%3A//www.google.com/reader/view/&iurl=http%3A//i2.ytimg.com/vi/5uZr3JWYdy8/hqdefault.jpg&sk=66CiOuZCC5A4J0KjrKP-pqbSeYv8BKxKC&use_get_video_info=1&load_modules=1&fs=1&feature=player_embedded&color1=0xb1b1b1&color2=0xcfcfcf&hl=en&cr=US&title=PaintRoll%27d&avg_rating=4.91150658216&length_seconds=103", "myContent", "400", "300", "9.0.0");
  });', array('type' => 'inline', 'weight' => 100));

  // Create the markup
  echo '<div id="myContent"></div>';

This looks good to me. For other methods of dealing with the aggregation or not of the external files I would wait for #352951: Make JS & CSS Preprocessing Pluggable to go in and allow for contrib modules to handle other cases. This patch is waiting on #363787: Plugins: Swappable subsystems for core.

The major short fall of this patch is the situation where you want to use an external version of jquery (via something like hook_js_alter) and want content to aggregate the js files. This is an edge case for the js system. If that is to be handled I would suggest a separate issue.

Hi!

I am looking for the external javascript to be called in from xslt file. Thus using the code in xsl file in head section of html section using script tag, not able to get sucess as the external file not called. Please do help me out:

var sc = document.createElement('script');
sc.setAttribute('src', 'http://maps.google.com/maps?file=api&v=2&key=ABQIAAAAt9dZ24lueXuymndHYfu...');
sc.setAttribute('type', 'text/javascript');
document.getElementsByTagName('head')[0].appendChild(sc);

Don't try to hijack cases, please. Setting back.

Angie asked for a different .js file other then Yahoo's, so we look to example.com!

Committed to HEAD. :) Thanks!!

Docs please!

I can't save the document for some reason.......

<li><a href="#drupal_add_js_external">External JavaScript can now be referenced through drupal_add_js()</a></li>

<h3 id="drupal_add_js_external">External JavaScript can now be referenced through drupal_add_js()</h3>
<a href="http://drupal.org/node/91250">(issue)</a> The <a href="http://api.drupal.org/api/function/drupal_add_js/7">drupal_add_js()</a> function can now add reference to external JavaScript files by passing a 'external' type.

Drupal 6.x:
<?php
drupal_set_html_head('<script type="text/javascript" src="http://example.com/example.js" />');
?>

Drupal 7.x:
<?php
drupal_add_js('http://example.com/example.js', 'external');
drupal_add_js('http://example.com/example.js', array('type' => 'external'));
?>

Also note: #219346: Cache External Javascript

I can't save the documentation either :(

It turned out that page was not being saved due to how PHP was compiled. Should be good now! Please add these updates!

I added Rob Loach's documentation from #96: http://drupal.org/node/224333#drupal_add_js_external

But I'm not sure what to do about http://drupal.org/node/394070, especially with regards to the '[]' markers in that document.

I'm having this problem suddenly with 6... tried disabling analytics but no dice.

Update: Shit. Wrong issue. Sorry!

Is there a backport for #94 for D6?

I would also like to know if this will get back ported to Drupal 6?

This will NOT be backported to Drupal 6.

This one saved me (for D6).

http://drupal.org/project/cache_external_files