Bug 1064395: concatenate and slightly minify javascript files

Bugzilla/Constants.pm

@@ -26,6 +26,8 @@ use Memoize;
 
     bz_locations
 
+    CONCATENATE_ASSETS
+
     IS_NULL
     NOT_NULL
 
@@ -210,6 +212,11 @@ use constant REST_DOC => "http://www.bugzilla.org/docs/tip/en/html/api/";
 use constant REMOTE_FILE => 'http://updates.bugzilla.org/bugzilla-update.xml';
 use constant LOCAL_FILE  => 'bugzilla-update.xml'; # Relative to datadir.
 
+# When true CSS and JavaScript assets will be concatanted and minified at
+# run-time, to reduce the number of requests required to render a page.
+# Setting this to a false value can help debugging.
+use constant CONCATENATE_ASSETS => 1;
+
 # These are unique values that are unlikely to match a string or a number,
 # to be used in criteria for match() functions and other things. They start
 # and end with spaces because most Bugzilla stuff has trim() called on it,

Bugzilla/Install/Filesystem.pm

@@ -31,6 +31,7 @@ use File::Path;
 use File::Basename;
 use File::Copy qw(move);
 use File::Spec;
+use File::Slurp;
 use IO::File;
 use POSIX ();
 
@@ -367,7 +368,7 @@ EOT
 
         "$assetsdir/.htaccess" => { perms => WS_SERVE, contents => <<EOT
 # Allow access to .css files
-<FilesMatch \\.css\$>
+<FilesMatch \\.(css|js)\$>
   Allow from all
 </FilesMatch>
 
@@ -410,6 +411,7 @@ sub update_filesystem {
 
     my $datadir = bz_locations->{'datadir'};
     my $graphsdir = bz_locations->{'graphsdir'};
+    my $assetsdir = bz_locations->{'assetsdir'};
     # If the graphs/ directory doesn't exist, we're upgrading from
     # a version old enough that we need to update the $datadir/mining 
     # format.
@@ -450,6 +452,13 @@ sub update_filesystem {
         _rename_file($oldparamsfile, "$datadir/$oldparamsfile");
     }
 
+    # Remove old assets htaccess file to force recreation with correct values.
+    if (-e "$assetsdir/.htaccess") {
+        if (read_file("$assetsdir/.htaccess") =~ /<FilesMatch \\\.css\$>/) {
+            unlink("$assetsdir/.htaccess");
+        }
+    }
+
     _create_files(%files);
     if ($params->{index_html}) {
         _create_files(%{$fs->{index_html}});
@@ -493,7 +502,7 @@ EOT
 
     _remove_empty_css_files();
     _convert_single_file_skins();
-    _remove_dynamic_css_files();
+    _remove_dynamic_assets();
 }
 
 sub _remove_empty_css_files {
@@ -538,10 +547,14 @@ sub _convert_single_file_skins {
     }
 }
 
-# delete all automatically generated css files to force recreation at the next
-# request.
-sub _remove_dynamic_css_files {
-    foreach my $file (glob(bz_locations()->{assetsdir} . '/*.css')) {
+# delete all automatically generated css/js files to force recreation at the
+# next request.
+sub _remove_dynamic_assets {
+    my @files = (
+        glob(bz_locations()->{assetsdir} . '/*.css'),
+        glob(bz_locations()->{assetsdir} . '/*.js'),
+    );
+    foreach my $file (@files) {
         unlink($file);
     }
 

Bugzilla/Template.pm

@@ -530,7 +530,7 @@ sub _concatenate_css {
         write_file($file, $content);
     }
 
-    $file =~ s/^\Q$cgi_path\E\///;
+    $file =~ s/^\Q$cgi_path\E\///o;
     return mtime_filter($file);
 }
 
@@ -543,6 +543,54 @@ sub _css_url_rewrite {
     return 'url(../../' . dirname($source) . '/' . $url . ')';
 }
 
+sub _concatenate_js {
+    return @_ unless CONCATENATE_ASSETS;
+    my ($sources) = @_;
+    return [] unless $sources && ref($sources);
+
+    my %files =
+        map {
+            (my $file = $_) =~ s/(^[^\?]+)\?.+/$1/;
+            $_ => $file;
+        } @$sources;
+
+    my $cgi_path   = bz_locations()->{cgi_path};
+    my $skins_path = bz_locations()->{assetsdir};
+
+    # build minified files
+    my @minified;
+    foreach my $source (@$sources) {
+        next unless -e "$cgi_path/$files{$source}";
+        my $file = $skins_path . '/' . md5_hex($source) . '.js';
+        if (!-e $file) {
+            my $content = read_file("$cgi_path/$files{$source}");
+
+            # minimal minification
+            $content =~ s#/\*.*?\*/##sg;    # block comments
+            $content =~ s#(^ +| +$)##gm;    # leading/trailing spaces
+            $content =~ s#^//.+$##gm;       # single line comments
+            $content =~ s#\n{2,}#\n#g;      # blank lines
+            $content =~ s#(^\s+|\s+$)##g;   # whitespace at the start/end of file
+
+            write_file($file, "/* $files{$source} */\n" . $content . "\n");
+        }
+        push @minified, $file;
+    }
+
+    # concat files
+    my $file = $skins_path . '/' . md5_hex(join(' ', @$sources)) . '.js';
+    if (!-e $file) {
+        my $content = '';
+        foreach my $source (@minified) {
+            $content .= read_file($source);
+        }
+        write_file($file, $content);
+    }
+
+    $file =~ s/^\Q$cgi_path\E\///o;
+    return [ $file ];
+}
+
 # YUI dependency resolution
 sub yui_resolve_deps {
     my ($yui, $yui_deps) = @_;
@@ -1054,6 +1102,7 @@ sub create {
 
             'css_files' => \&css_files,
             yui_resolve_deps => \&yui_resolve_deps,
+            concatenate_js => \&_concatenate_js,
 
             # All classifications (sorted by sortkey, name)
             'all_classifications' => sub {

template/en/default/global/header.html.tmpl

@@ -90,8 +90,16 @@
     [% SET yui = yui_resolve_deps(yui, yui_deps) %]
 
     [% SET css_sets = css_files(style_urls, yui, yui_css) %]
-    <link href="[% css_sets.unified_standard_skin FILTER html %]"
-          rel="stylesheet" type="text/css">
+    [% IF constants.CONCATENATE_ASSETS %]
+      [% PROCESS format_css_link asset_url = css_sets.unified_standard_skin %]
+    [% ELSE %]
+      [% FOREACH asset_url = css_sets.standard %]
+        [% PROCESS format_css_link %]
+      [% END %]
+      [% FOREACH asset_url = css_sets.skin %]
+        [% PROCESS format_css_link %]
+      [% END %]
+    [% END %]
 
     [% IF style %]
       <style type="text/css">
@@ -100,8 +108,13 @@
     [% END %]
 
     [% IF css_sets.unified_custom %]
-      <link href="[% css_sets.unified_custom FILTER html %]"
-            rel="stylesheet" type="text/css">
+      [% IF constants.CONCATENATE_ASSETS %]
+        [% PROCESS format_css_link asset_url = css_sets.unified_custom %]
+      [% ELSE %]
+        [% FOREACH asset_rul = css_sets.custom %]
+          [% PROCESS format_css_link %]
+        [% END %]
+      [% END %]
     [% END %]
 
     [%# YUI Scripts %]
@@ -110,7 +123,7 @@
     [% END %]
     [% starting_js_urls.push('js/global.js') %]
 
-    [% FOREACH javascript_url = starting_js_urls %]
+    [% FOREACH asset_url = concatenate_js(starting_js_urls) %]
       [% PROCESS format_js_link %]
     [% END %]
 
@@ -180,7 +193,7 @@
     // -->
     </script>
 
-    [% FOREACH javascript_url = javascript_urls %]
+    [% FOREACH asset_url = concatenate_js(javascript_urls) %]
       [% PROCESS format_js_link %]
     [% END %]
 
@@ -251,6 +264,10 @@
       <div id="message">[% message %]</div>
     [% END %]
 
+[% BLOCK format_css_link %]
+  <link href="[% asset_url FILTER html %]" rel="stylesheet" type="text/css">
+[% END %]
+
 [% BLOCK format_js_link %]
-  <script type="text/javascript" src="[% javascript_url FILTER mtime FILTER html %]"></script>
+  <script type="text/javascript" src="[% asset_url FILTER mtime FILTER html %]"></script>
 [% END %]