Bug 977969: concatenate and slightly minify css files

.gitignore

@@ -10,5 +10,6 @@
 /localconfig
 /index.html
 
+/skins/assets
 /skins/contrib/Dusk/admin.css
 /skins/contrib/Dusk/bug.css

Bugzilla/Install/Filesystem.pm

@@ -199,6 +199,8 @@ sub FILESYSTEM {
                                   dirs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE },
          "$datadir/db"      => { files => CGI_WRITE,
                                   dirs => DIR_CGI_WRITE },
+         "$skinsdir/assets" => { files => WS_SERVE,
+                                  dirs => DIR_CGI_OVERWRITE | DIR_ALSO_WS_SERVE },
 
          # Readable directories
          "$datadir/mining"     => { files => CGI_READ,
@@ -269,6 +271,7 @@ sub FILESYSTEM {
         $attachdir              => DIR_CGI_WRITE,
         $graphsdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
         $webdotdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
+        "$skinsdir/assets"      => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
         # Directories that contain content served directly by the web server.
         "$skinsdir/custom"      => DIR_WS_SERVE,
         "$skinsdir/contrib"     => DIR_WS_SERVE,
@@ -475,6 +478,7 @@ EOT
 
     _remove_empty_css_files();
     _convert_single_file_skins();
+    _remove_dynamic_css_files();
 }
 
 sub _remove_empty_css_files {
@@ -519,6 +523,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()->{skinsdir} . '/assets/*.css')) {
+        unlink($file);
+    }
+}
+
 sub create_htaccess {
     _create_files(%{FILESYSTEM()->{htaccess}});
 

Bugzilla/Template.pm

@@ -26,9 +26,11 @@ use Bugzilla::Token;
 use Cwd qw(abs_path);
 use MIME::Base64;
 use Date::Format ();
+use Digest::MD5 qw(md5_hex);
 use File::Basename qw(basename dirname);
 use File::Find;
 use File::Path qw(rmtree mkpath);
+use File::Slurp;
 use File::Spec;
 use IO::Dir;
 use List::MoreUtils qw(firstidx);
@@ -422,10 +424,12 @@ sub mtime_filter {
 
 # Set up the skin CSS cascade:
 #
-#  1. YUI CSS
-#  2. Standard Bugzilla stylesheet set (persistent)
-#  3. Third-party "skin" stylesheet set, per user prefs (persistent)
-#  4. Custom Bugzilla stylesheet set (persistent)
+#  1. standard/global.css
+#  2. YUI CSS
+#  3. Standard Bugzilla stylesheet set
+#  4. Third-party "skin" stylesheet set, per user prefs
+#  5. Inline css passed to global/header.html.tmpl
+#  6. Custom Bugzilla stylesheet set
 
 sub css_files {
     my ($style_urls, $yui, $yui_css) = @_;
@@ -448,7 +452,12 @@ sub css_files {
             push(@{ $by_type{$key} }, $set->{$key});
         }
     }
-    
+
+    # build unified
+    $by_type{unified_standard_skin} = _concatenate_css($by_type{standard},
+                                                       $by_type{skin});
+    $by_type{unified_custom} = _concatenate_css($by_type{custom});
+
     return \%by_type;
 }
 
@@ -456,30 +465,85 @@ sub _css_link_set {
     my ($file_name) = @_;
 
     my %set = (standard => mtime_filter($file_name));
-    
-    # We use (^|/) to allow Extensions to use the skins system if they
-    # want.
-    if ($file_name !~ m{(^|/)skins/standard/}) {
+
+    # We use (?:^|/) to allow Extensions to use the skins system if they want.
+    if ($file_name !~ m{(?:^|/)skins/standard/}) {
         return \%set;
     }
 
     my $skin = Bugzilla->user->settings->{skin}->{value};
     my $cgi_path = bz_locations()->{'cgi_path'};
     my $skin_file_name = $file_name;
-    $skin_file_name =~ s{(^|/)skins/standard/}{skins/contrib/$skin/};
+    $skin_file_name =~ s{(?:^|/)skins/standard/}{skins/contrib/$skin/};
     if (my $mtime = _mtime("$cgi_path/$skin_file_name")) {
         $set{skin} = mtime_filter($skin_file_name, $mtime);
     }
 
     my $custom_file_name = $file_name;
-    $custom_file_name =~ s{(^|/)skins/standard/}{skins/custom/};
+    $custom_file_name =~ s{(?:^|/)skins/standard/}{skins/custom/};
     if (my $custom_mtime = _mtime("$cgi_path/$custom_file_name")) {
         $set{custom} = mtime_filter($custom_file_name, $custom_mtime);
     }
-    
+
     return \%set;
 }
 
+sub _concatenate_css {
+    my @sources = map { @$_ } @_;
+    return unless @sources;
+
+    my %files =
+        map {
+            (my $file = $_) =~ s/(^[^\?]+).+/$1/;
+            $_ => $file;
+        } @sources;
+
+    my $cgi_path   = bz_locations()->{cgi_path};
+    my $skins_path = bz_locations()->{skinsdir};
+
+    # build minified files
+    my @minified;
+    foreach my $source (@sources) {
+        next unless -e "$cgi_path/$files{$source}";
+        my $file = $skins_path . '/assets/' . md5_hex($source) . '.css';
+        if (!-e $file) {
+            my $content = read_file("$cgi_path/$files{$source}");
+
+            # minify
+            $content =~ s{/\*.*?\*/}{}sg;   # comments
+            $content =~ s{(^\s+|\s+$)}{}mg; # leading/trailing whitespace
+            $content =~ s{\n}{}g;           # single line
+
+            # rewrite urls
+            $content =~ s{url\(([^\)]+)\)}{_css_url_rewrite($source, $1)}eig;
+
+            write_file($file, "/* $files{$source} */\n" . $content . "\n");
+        }
+        push @minified, $file;
+    }
+
+    # concat files
+    my $file = $skins_path . '/assets/' . md5_hex(join(' ', @sources)) . '.css';
+    if (!-e $file) {
+        my $content = '';
+        foreach my $source (@minified) {
+            $content .= read_file($source);
+        }
+        write_file($file, $content);
+    }
+
+    return mtime_filter($file);
+}
+
+sub _css_url_rewrite {
+    my ($source, $url) = @_;
+    # rewrite relative urls as the unified stylesheet lives in a different
+    # directory from the source
+    $url =~ s/(^['"]|['"]$)//g;
+    return $url if substr($url, 0, 1) eq '/';
+    return 'url(../../' . dirname($source) . '/' . $url . ')';
+}
+
 # YUI dependency resolution
 sub yui_resolve_deps {
     my ($yui, $yui_deps) = @_;

skins/README

-There are three directories here, standard/, custom/, and contrib/.
+There are four directories here, standard/, custom/, contrib/, and assets/.
 
-standard/ holds the standard stylesheets. These are used no matter
-what skin the user selects. If the user selects the "Classic" skin,
-then *only* the standard/ stylesheets are used.
+standard/ holds the standard stylesheets. These are used no matter what skin
+the user selects. If the user selects the "Classic" skin, then *only* the
+standard/ stylesheets are used.
 
-contrib/ holds "skins" that the user can select in their preferences.
-skins are in directories, and they contain files with the same names
-as the files in skins/standard/. Simply putting a new directory
-into the contrib/ directory adds a new skin as an option in users'
-preferences.
+contrib/ holds "skins" that the user can select in their preferences.  skins
+are in directories, and they contain files with the same names as the files in
+skins/standard/. Simply putting a new directory into the contrib/ directory
+adds a new skin as an option in users' preferences.
 
-custom/ allows you to locally override the standard/ and contrib/ CSS.
-If you put files into the custom/ directory with the same names as the CSS
-files in skins/standard/, you can override the standard/ and contrib/
-CSS. For example, if you want to override some CSS in
-skins/standard/global.css, then you should create a file called "global.css"
-in custom/ and put some CSS in it. The CSS you put into files in custom/ will
-be used *in addition* to the CSS in skins/standard/ or the CSS in
-skins/contrib/. It will apply to every skin.
+custom/ allows you to locally override the standard/ and contrib/ CSS.  If you
+put files into the custom/ directory with the same names as the CSS files in
+skins/standard/, you can override the standard/ and contrib/ CSS. For example,
+if you want to override some CSS in skins/standard/global.css, then you should
+create a file called "global.css" in custom/ and put some CSS in it. The CSS
+you put into files in custom/ will be used *in addition* to the CSS in
+skins/standard/ or the CSS in skins/contrib/. It will apply to every skin.
+
+assets/ holds the minified and concatenated files which are created by
+checksetup.pl and Bugzilla::Template.  Do not edit the files in this directory.

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

@@ -90,35 +90,20 @@
     [% PROCESS 'global/setting-descs.none.tmpl' %]
 
     [% SET yui = yui_resolve_deps(yui, yui_deps) %]
-    [% SET css_sets = css_files(style_urls, yui, yui_css) %]
-
-    [%# CSS cascade, parts 1 & 2: YUI & Standard Bugzilla stylesheet set (persistent).
-      # Always present. %]
-    <link href="[% 'skins/standard/global.css' FILTER mtime FILTER html %]"
-          rel="alternate stylesheet" 
-          title="[% setting_descs.standard FILTER html %]">
-    [% FOREACH style_url = css_sets.standard %]
-      [% PROCESS format_css_link css_set_name = 'standard' %]
-    [% END %]
 
-    [%# CSS cascade, part 3: Third-party stylesheet set, per user prefs. %]
-    [% FOREACH style_url = css_sets.skin %]
-      [% PROCESS format_css_link css_set_name = user.settings.skin.value %]
-    [% END %]
+    [% SET css_sets = css_files(style_urls, yui, yui_css) %]
+    <link href="[% css_sets.unified_standard_skin FILTER html %]"
+          rel="stylesheet" type="text/css">
 
-    [%# CSS cascade, part 4: page-specific styles. %]
     [% IF style %]
       <style type="text/css">
         [% style %]
       </style>
     [% END %]
 
-    [%# CSS cascade, part 5: Custom Bugzilla stylesheet set (persistent).
-      # Always present. Site administrators may override all other style
-      # definitions, including skins, using custom stylesheets.
-      #%]
-    [% FOREACH style_url = css_sets.custom %]
-      [% PROCESS format_css_link css_set_name = 'standard' %]
+    [% IF css_sets.unified_custom %]
+      <link href="[% css_sets.unified_custom FILTER html %]"
+            rel="stylesheet" type="text/css">
     [% END %]
 
     [%# YUI Scripts %]
@@ -265,19 +250,6 @@
       <div id="message">[% message %]</div>
     [% END %]
 
-[% BLOCK format_css_link %]
-  [% IF css_set_name == 'standard' %]
-    [% SET css_title_link = '' %]
-  [% ELSE %]
-    [% css_title_link = BLOCK ~%]
-      title="[% setting_descs.${user.settings.skin.value} || user.settings.skin.value FILTER html %]"
-    [% END %]
-  [% END %]
-
-  <link href="[% style_url FILTER html %]" rel="stylesheet"
-        type="text/css" [% css_title_link FILTER none %]>
-[% END %]
-
 [% BLOCK format_js_link %]
   <script type="text/javascript" src="[% javascript_url FILTER mtime FILTER html %]"></script>
 [% END %]