Query string appended drupal_add_js breaks certain scripts
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | patch (code needs work) |
In the 'default' case in drupal_get_js appends a cache-busting query string to URLs in an attempt to get browsers to reload pages when page caches are updated. This is fine, except for the fact that URLs added by modules may already have query-strings as part of the URI.
For instance, when using the Gallery module and navigating to the Gallery plugin management page, this URI is added via drupal_add_js():
/index.php?q=gallery&g2_view=core.CombinedJavascript&g2_key=cd60d0c257ddd99cc07f4efafd8004b0
drupal_get_js() appends "?1207482595". Since the URI above already has a query-string, the ? should be replaced with a &.
In some circumstances, this usage of ? instead of & causes actual breakage because the webserver chooses to disregard all of the query parameters after the first ? and only acknowledges the ones after the second ?.
Attached is a patch which will test the URL before appending the query string and determine which parameter should be used.
| Attachment | Size |
|---|---|
| includes_common.inc_.diff | 1.33 KB |
#1
Apply the same logic to drupal_get_css(). I haven't seen an issue here, yet, but it's begging for it to occur.
#2
+1
Arbitrary query strings can also break certain JavaScripts, for example TinyMCE, as mentioned in http://drupal.org/node/218296#comment-810963
The workaround for now is to set the $cache parameter to FALSE -- however, this results in browsers not caching the JavaScript at all.
So, in addition to those patches here, we badly need a way to force no query string at all for certain drupal_add_js() calls.
Marking as critical.
#3
I also came across this issue in Drupal 6 - see #242875: Admin option to disable dummy query-string appended to css and js file links. Have marked that as a duplicate of this one.
#4
This issue also breaks language pack loading for the tinyMCE module. The Language packs javascript url returns a drupal 404 error, thus the javascript is never loaded. This should be patched for drupal 6.x as well.
#5
Bugs are fixed in HEAD first.
#6
Reroll for D7.
#7
For D7, I'd suggest to allow developers to pass
$options = array('query' => FALSE)in the new $options array to disable the query string for certain JS files only.Also, this issue unfortunately seems to deal with two issues:
a) Original poster: Drupal's query string is not properly added to an existing query string, i.e.
myfile.js?arg1=value?B. There are two ? in the resulting URL. The original poster needs a way to properly assign a query string, f.e.<?phpdrupal_add_js($filename, array('query' => 'arg1=value'));
?>
b) Drupal's query string breaks the execution of certain scripts, i.e.
myfile.js?B, is sufficient to break the execution of certain scripts (perhaps in specific browsers only, dunno). Since setting the 'cache' option to FALSE results in yet another query string ($info['cache'] ? $query_string : '?' . REQUEST_TIME), there should be an option for developers to disable Drupal's query cache string for certain scripts only, f.e.<?phpdrupal_add_js($filename, array('query cache' => FALSE));
?>
#8
I seem to have the same problem. I'm new to drupal and all the lingo. Here's a screen shot that explains what I get when I try to upload an image using Tinymce and the wysiwygAPI. I also had the same result when trying to get Image assist to work without the error messages displayed.
Also I've managed to get simple patches working before but I'm having trouble understanding which lines to add or remove on this one.
#9
Bugs are fixed in the current development branch first.
@sodafox: Also, please do not change issue classification. The bug you are experiencing seems to be limited to using Wysiwyg API and Image Assist. If you were experiencing the bug of *this* issue, neither TinyMCE nor Image Assist would show up at all. Please file a new issue in Image Assist's queue (but search for existing issues first). Thanks.