Hello, I just want to give you some feedback about this very recent -and useful- module:

- I have tried it in drupal 5. Works fine, no problem up until now (users haven't reported any issue)
- I think you could add a small note in the readme, stating that the path for the folder containing css and js files must be relative to the server, not to drupal, as this module uses functions available to php (and this is important when hosted in a shared server)

- I think it would be useful to modify load.php code to allow for different folders for css and js files. For instance, if you use aggregtated and compressed css files in Drupal, and the module javascript_aggregator, you will probably end up with css and js files in different paths. Here is what I have changed for my site:

// Read the requested file name from GET parameter
$file = @$_GET['file'] or debug_exit('Missing file parameter');


//Modified to allow different path for js files
if (preg_match('/\.(css)$/i', $file, $file_type)) {
        $file = $baseDirCss.$file;
} elseif (preg_match('/\.(js)$/i', $file, $file_type)) {
        $file = $baseDirJs.$file;
} else {
        debug_exit($file." is not a javascript or css file.");
}         


//$file = $baseDir.$file;



// Check file extension
//if (!preg_match('/\.((js)|(css))$/i', $file, $file_type)) {
//	debug_exit($file." is not a javascript or css file.");
//}


// End of changes

Of course, I have added $baseDirCss and $baseDirJs variables at the top of file, and have copied .htaccess in both folders.

BTW, great module, I have been looking around for some time trying to achieve this simple but great improvement to site performance

CommentFileSizeAuthor
#6 README.txt5.23 KBgremlinc5

Comments

gremlinc5’s picture

Glad to read someone is actually using it!

Thanks for the note about the Readme. I'll add a better explanation ASAP.

About your request:
it's fairly easy to implement it, but I don't understand the use case.

I mean: the $baseDir parameter is relative to load.php (or absolute to your server root) and has to point to your site's root.

Example:
If you have drupal installed in the /drupal/ directory and there is a link to /drupal/misc/drupal.js, the $baseDir must be "../../../" (one for /drupal, one for /modules, one for /smartcache).

I do not see how you could have a different $baseDir for js and css files (but I do not know where JS Aggregator stores its files).

If you could explain a use case, I would implement the patch with pleasure.

Thank you for the feedback!

gremlinc5’s picture

BTW, maybe a slightly more efficient methos is (not yet tested):

// Read the requested file name from GET parameter
$file = @$_GET['file'] or debug_exit('Missing file parameter');
// $file = $baseDir.$file;

// Check file extension
if (!preg_match('/\.((js)|(css))$/i', $file, $file_type)) {
	debug_exit($file." is not a javascript or css file.");
}

$file_type = strtolower($file_type[1]);

// Implementation of different base paths
$file = ($file_type === "js") ? $baseDirJs.$file : $baseDirCss.$file;

Because you don't have to eval two regexp for each *.js request.

gremlinc5’s picture

Deleted double post.

pedropablo’s picture

mmmm, maybe I did something wrong...

I had to include both basedir because when debugging I saw that load.php was looking for files without the whole path. Maybe that happeded because I placed the .htaccess file not in the root of the server, but in the folder were css files were stored, so .htaccess passed the file name without the path to php.load (I preferred to add it just in that folder to avoid checks for every request, and also to avoid breaking anything)

Reading so is clear I have little experience dealing with both redirects and php... :-))

So maybe my use case would improve performance just a bit, but I don't think the added setup complexity worths it... maybe just when you only have all css and js files stored in their respective folders, just like I have due to aggregation and compression.
In any other case, setup would be impossible. It worked for me just because of this rare coincidence!!

Maybe it worths to mention something about this in the readme to make sure every file is in its place when setting up the module, and that's all

thank you for your time. You have help me better understand what I am doing... :-)

pedropablo’s picture

Thank you for the improvement (too advanced solution for my little php skills :-). I will test it

gremlinc5’s picture

StatusFileSize
new5.23 KB

I see. I did not specified in which .htaccess you should have placed redirection.

I slightly edited readme.txt, adding some examples on baseDir configuration, so you might want to check them out.

Thank you again!

pedropablo’s picture

Much more clear now. Thank you!!

gremlinc5’s picture

Status: Active » Fixed
gremlinc5’s picture

Assigned: Unassigned » gremlinc5
Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.