Incompatibility with Javascript Aggregator

Sounds like I need to change the weight on my module so it runs near the end.

Hmmm, I didn't look into the details. I just changed the weight to 10 and saw no change. JS aggregated files are not being rewritten.

Ah, on further research it looks like it needs to fire after jquery update which has a weight of 99. When I set the weight of Parallel to 100 it works as expected.

Turns out that jQuery Update is also manipulating the theme registry in tricky ways. And we have to do the same so that our code runs last. The code in this patch is taken almost verbatim from jQuery Update.

changed update 6001 to 6100
added in a hook install function
committed

I'm still having an issue with 6.x-1.0-beta2.

Parallel enabled - JS is aggregated but not compressed.

Parallel disabled - JS is compressed and I get a file with .jsmin.js attached to the file name.

I can confirm that, same problem by me.

Subscribing

I moved <?php echo $scripts ?> to bottom of my template (page.tpl.php) just before closure.
JavaScript from header aren't minified. But those in footer are.
Both URL point to my CDN server.

I tried some debug traces with some "echo".

function javascript_aggregator_preprocess_page(&$variables) {
  // Only do this for pages that have JavaScript on them.
  if (!empty($variables['scripts'])) {
echo 'javascript_aggregator_preprocess_page';
    $variables['scripts'] = _javascript_aggregator_minify($variables['scripts']);
  }
}

function phptemplate_closure($main = 0) {
  $footer = module_invoke_all('footer', $main);
  
  $js_footer = drupal_get_js('footer');
  // Only do this for pages that have JavaScript on them.
  if (!empty($js_footer)) {
echo 'phptemplate_closure';
    $js_footer = _javascript_aggregator_minify($js_footer);
  }

  return implode("\n", $footer) . $js_footer;
}

function _javascript_aggregator_minify($scripts) {
echo $scripts;
....
}

And here is interesting result :

phptemplate_closure
<script type="text/javascript" src="/sites/default/files/js/js_6dce5c63a61c264ce1dfa4a44885c864.js"></script>

javascript_aggregator_preprocess_page
javascript_aggregator_preprocess_page<script type="text/javascript" src="//cdn3.example.com/sites/default/files/js/js_7e290aa7e34d227e44099a687fa11098.js">
</script>

Javascript_Aggregator search pattern could not operate correctly in second case because URL has already been replaced by Parallel module.

Are we sure Parallel module always operates after Javascript_Aggregator ?

Maybe this code from Javascript_Aggregator causes this issue :

/**
 * Implementation of hook_theme_registry_alter().
 *
 * Make javascript_aggregator's page preprocess function run *after* everything else's (even jQuery Update).
 */
function javascript_aggregator_theme_registry_alter(&$theme_registry) {
  if (isset($theme_registry['page'])) {
    // If javascript_aggregator's preprocess function is there already, remove it.
    if ($key = array_search('javascript_aggregator_preprocess_page', $theme_registry['page']['preprocess functions'])) {
      unset($theme_registry['page']['preprocess functions'][$key]);
    }
    // Now tack it on at the end so it runs after everything else.
    $theme_registry['page']['preprocess functions'][] = 'javascript_aggregator_preprocess_page';
  } 
}

Modules weights have no effect because Javascript_Aggregator forces to be always called last one.
Issue to be assigned to Javascript_Aggregator module ?
Hmmm no DEV release since July 2009, I'm afraid it will not be fixed rapidly.

Is it possible to fixed it in another way in Parallel module ?

I found a solution with modified Javascript_Aggregator code :

In javascript_aggregator.module file (function _javascript_aggregator_minify($scripts))
replace :

    $pattern = "!(<script type=\"text\/javascript\" src=\"$path_to_files_directory)(.*?)(\"(.*?)><\/script>)!";
    if (preg_match_all($pattern, $scripts, $matches) > 0) {
      $aggregated_file_name = $matches[2][0];

by

    $pattern = "!(<script type=\"text\/javascript\" src=\"(.*?)$path_to_files_directory)(.*?)(\"(.*?)><\/script>)!";
    if (preg_match_all($pattern, $scripts, $matches) > 0) {
      $aggregated_file_name = $matches[3][0];

I can confirm that the "patch" proposed by srobert72 works very well on my test site!

@SaxxIng
Thank you for testing my patch.

Problem remains this patch is for Javascript_Aggregator and not Parallel module.

We could reassign this issue to Javascript_Aggregator module.
But no DEV release since nearly one year for Javascript_Aggregator module. And I don't know if maintainer would want to fixe something concerning another module.
That's why I leave this issue to Parallel module.

EDIT on April 13, 2010
Javascript_Aggregator module has just released new DEV version.
I was wrong thinking there's no more activity in this module.
I proposed to patch it in order to fix issue with Parallel module.
See #770374: Incompatible with Parallel module

I believe that you just need to replicate the approach that I took in the patch that I submitted. The problem in that case was jQuery Update. Parallel's theme functions needs to run after jQuery Update. This was accomplished with a combination of setting the module weight to one heavier than jQuery Update, and use hook_theme_registry_alter() to move Parallel's theme function to be last. To make this approach work with JavaScript Aggregator all you should need to do is to make the weight of Parallel to be one heavier than JavaScript Aggregator (Parallel's implementation of hook_theme_registry_alter() just needs to run after JavaScript Aggregator's). The weight of JavaScript Aggregator is 9999 so setting the weight of Parallel to 10000 should do the trick.

This feature has been added to Javascript_Aggregator module.
It now proposes option to serve JS files from external site.

#533190: PATCH: allow serving js files from an external file server

Solution without any patch is to configure base URL in Parallel module to serve images and CSS, and base URL in Javascript_Aggregator to serve JS files.

EDIT on April 13, 2010
Sorry for wrong news, this Javascript_Aggregator patch is for 5.x branch and not 6.x

Well that's a bit crufty, but oh well.

@mikeytown2
What solution do you prefer ?

hmmm... I have some code that makes parallel compatible with boost; looks like I need to modify boost for the new JavaScript Aggregator patch.

EDIT #15
Sorry for wrong news, this Javascript_Aggregator patch is for 5.x branch and not 6.x

EDIT #13
Javascript_Aggregator module has just released new DEV version.
I was wrong thinking there's no more activity in this module.
I proposed to patch it in order to fix issue with Parallel module.
See #770374: Incompatible with Parallel module

I committed the code suggested in #11 to Javascript Aggregator. Please try new dev version and report here: #770374: Incompatible with Parallel module

Thanks.

Patch commited in Javascript_Aggregator module works properly.
Seems this issue could be closed.

reopen if something new happens

The patch mentioned in #20 and #21 was either never committed to JavaScript Aggregator, or it was rolled back. This is a good thing because JS Aggregator is the wrong place to make such a change (Any module should only be doing one thing).

To get Parallel working with JS Aggregator simply requires the weight of Parallel to be heavier. It must be 10000.

I would write a patch to do this, but I think it would be easier for @mikeytown2 to just edit the .install file and change the weight from 100 to 10000.

Here's the query

db_query("UPDATE {system} SET weight = 10000 WHERE name = 'parallel'");

Committed
http://drupal.org/project/cvs/548828

@dalin #24
Patch is still in CVS (line 102):
http://drupalcode.org/viewvc/drupal/contributions/modules/javascript_agg...

I don't understand why it was wrong place because Javascript_Aggregator continues to do just its work without any interaction with Parallel. Any trace of Parallel in Javascript_Aggregator code.

Before Mikeytown2's patch in #25, the current stable version of Parallels + the current stable version of JavaScript Aggregator resulted in the JS file being moved to the CDN, but it was not minified.

One of the rules of good programming is that each component should do just one atomic thing and should not be affected by enabling/disabling other components. If you either build a component to do X, Y, Z and wash your dishes you quickly get into a maintenance nightmare. If you have to create workarounds for other components you also get into a maintenance nightmare. The logical way to have the system work is for Parallels to fire last to rewrite finalized URLs from the local system to the remote system. JSA should not need to write preg_replace workarounds for cases when Parallels is or isn't installed.

And don't you think it could be a limitation in JSA to always search for relative URL ?
Doing this it will never be compatible with other modules which rewrite URL such CDN like modules.

Changing weight is a sort of workaround you speak about. You set your weight depending on other's weight.

I agree your solution works and it's a good one. Don't misunderstand me.
But I'm not sure mine was so bad. I really think JSA couldn't operate in all cases depending on which modules operate before it.

For information there is same issue in ColorBox module. I've just checked it has weight 0.
#773006: Add support for the Parallel module

@dalin #24
The patch mentioned above did get committed to 6.x-1.x-dev of Javascript Aggregator! Since it's a minor change I dont think it needs to be rolled back?

You are also right, about #788202: The current module weight is absurdedly high.

so, change this to 110?

@mikeytown2 I would say no, the current regex workaround that JSA is trying to use to make itself run after Parallel isn't working.

@dalin
But it works for me and for SaxxIng #12...
You said it hasn't been commited, but it was.
Are you sure you have this piece of code in your Drupal code ?
Check .../modules/javascript_aggregator/javascript_aggregator.module line 102

JSA 6.1.x dev has some regex that should work, but the current release does not.

Yes and weight patch for Parallel has been commited to DEV branch too.
We are not comparing official releases.
But we are searching right solution in DEV branches before making it official.

In fact, this all in 6.x-1.x-dev (only!) at the moment.

Hrrrrm. Just noticed that all of a sudden my aggregated CSS and JS have absolute URLs. Not sure what is causing this or why. I don't think it's core, but I don't have any modules on this site that I suspect either. Well here's a patch that handles both relative and absolute paths for CSS and JS.

patch file didn't apply using CVS for some strange reason... my guess is the 2 random \ No newline at end of file lines in there.

can you verify that this patch does what your looking for?

eh looks good
committed.

sub