Core patch improvements

Thanks a lot for your feedback! :)

1) Will look into this ASAP.
2) I planned that as a future feature. Will look into this as well.
3) I disagree here. CDN cron typically takes a lot of time (relatively), and for that reason, it deserves a separate cron run, so it doesn't slow down the normal cron.

(I preferred to reply faster and less complete. Will answer #1 and #2 later today.)

#3: Excellent idea! Implemented it right away, see my latest CVS commit.

#1: static_file() is a bad name. I'd opt for file_url(), which is much more descriptive. Other than that, I agree.

#2: you're not quite right here, I think. You're saying that your patch from #1 has the ability to try multiple servers, i.e.:

• File exists on CDN? Serve from CDN and stop, else continue
• File exists on static file server? Serve from static file server and stop, else continue
• Serve from Drupal's web server

I agree that this is a necessity, but your patch doesn't implement this yet as far as I can tell.

Files are static content.

I've already replied to your comment, too.

Those aren't issues.

- JS in header delays the rendering of the page. I'll be trying to get D7 to put it in the footer as well. Until somebody
- Compression has been *removed* from Drupal 6 core because it's so problematic. The only compressors that don't result in problems at the moment are the YUI compressor and Dojo Shrinksafe, but both require the Rhine Javascript engine, which is written in Java, so we can't depend on it for Drupal core.
- system_performance_settings: I didn't write this, I didn't modify this part of the D5 backport of the JS aggregation patch.

I'll add static_file_url().

1) It /seems/ controversial, but it's a solid decision. It just breaks JS in a lot of cases. Even Drupal core's jquery.js file, which is compressed with the packer, is often problematic.

2) Incorrect. 95% of all modules don't set the location argument. Simply changing the default will thus affect all of them. This has nothing to do with JS compression.
In my personal experience, and I use a fair amount of modules that ship with JS, it works for all of them. :)
And making it a separate patch makes sense, but then you'd have to apply two patches. One for file URL rewriting + JS aggregation, and one for changing the JS location from header to footer. That's why I opted for a single patch. The added benefit is that people actually *test* if it (i.e. JS location) breaks any JS out there, so I can choose to change my strategy.

3) I didn't notice this had changed in 5.5. The JS aggregation backport patch was initially against 5.1, so that's why I guess. :) Will fix!

update.php is only used by admins, so optimizations there are unnecessary, which means I did this by intention.

The scope musn't change in drupal_get_js(), but in drupal_add_js(), in which it *is* changed.

I know it's unsure what the results will be, but I couldn't produce any issues. I guess I should keep drupal.js and jquery.js in the header, for the sake of compatibility with modules that add it to header. But at least the google_analytics can (and even should be) put in the footer. Most people add it to the header out of habit.

Not actually a patch, but a screenshot that's the result of my patch.

I'm rewriting the patch once again, to provide an UI to configure the file servers (that's what I decided to call them in the end).

Things I've learned:
- Provide 2 new hooks instead of hacky $conf array entries: hook_file_server() and hook_file_server_info.
- "file server" is the best thing I can come up with, and the easiest to understand for the average Drupal user. Feel free to make suggestions.
- The static file server shouldn't be supported by Drupal core. It'd be too advanced for the average Drupal user, as illustrated by the attached screenshot.

More patches will follow later. I'd like to get some initial feedback first.

Improved UI:
- Better text.
- No more static file server in Drupal core by default.
- The default file server - the web server itself - is now always displayed as the last file server, i.e. the server that will always be fallen back to.

There's a very good reason actually IMO: usability. A large part of Drupal users don't know what a static file server is, or at least shouldn't be bothered with it.

Another reason is that you may want to write the static file server module in such a way that you can use *several* of them, choosing by filetype (e.g. streaming video versus images) which server in an array of servers should be synced too/served from.

Well, then I will leave it out for the sake of simplicity, so I can get it in Drupal core faster. Static file server support could still be an easy follow-up patch.

Attached is the updated patch I just committed. It provides a single new hook: hook_file_server().

I want to keep the core patch as simple as possible - I've said this before. It's not just two lines. It's a new feature, with usability consequences and so forth. I won't change my mind, so please stop asking for it.

And I will most definitely not include the static file serving feature in the CDN integration module. That's completely beyond its scope.

I have implemented everything you've asked for in this issue. Please come up with *constructive* feedback about the current core patch.

1) It's not cdn module's task to provide a fallback mechanism. It's Drupal core's. And that's what the core patch allows you to do. So this is already solved.
2) I've explained it multiple times already now. I won't repeat myself again.

Marking as fixed, since the original requests have all been implemented.

Apparently you're incapable of reading what I write. I've repeatedly said that the design is that the file_url() function calls a module's hook implementation (e.g. the CDN module's), and if FALSE instead of an URL is returned, then it will try the next server.

See the code from the patch:

// Try the first preferred file server to check if it can serve the file.
// Then try the second, and so on.
foreach ($modules as $module) {
  $file_url = module_invoke($module, 'url', $file_path, $absolute_url);

  // If the URL rewriting function sucesfully generated a URL, we can stop
  // trying to find a server that can serve the file.
  if ($file_url) {
    break;
  }
}

I just discovered that somehow, I failed to commit the change to cdn_file_url(). Why couldn't you just point me to that, instead of saying that the design has problems?

Created an issue in the issue queue of the Drupal project: http://drupal.org/node/214934.