Last Comment Bug 1064395 - concatenate and slightly minify javascript files
: concatenate and slightly minify javascript files
Status: RESOLVED FIXED
: bmo-goal
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: All All
-- normal (vote)
: Bugzilla 5.0
Assigned To: Byron Jones ‹:glob›
: default-qa
:
Mentors:
Depends on:
Blocks: 1072110
  Show dependency treegraph
 
Reported: 2014-09-08 10:11 PDT by Byron Jones ‹:glob›
Modified: 2015-02-21 12:01 PST (History)
1 user (show)
glob: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
1064678_1.patch (684 bytes, patch)
2014-09-09 22:50 PDT, Byron Jones ‹:glob›
no flags Details | Diff | Splinter Review
1064395_1.patch (7.87 KB, patch)
2014-09-10 09:05 PDT, Byron Jones ‹:glob›
dkl: review-
Details | Diff | Splinter Review
1064395_2.patch (7.88 KB, patch)
2014-09-17 01:15 PDT, Byron Jones ‹:glob›
dkl: review+
Details | Diff | Splinter Review

Description User image Byron Jones ‹:glob› 2014-09-08 10:11:19 PDT
similar to what we do for css, we should concatenate and slightly minify javascript files.
Comment 1 User image Byron Jones ‹:glob› 2014-09-09 22:50:37 PDT
Created attachment 8486976 [details] [diff] [review]
1064678_1.patch

- concatenates starting_js_urls and javascript_urls into individual requests
- performs minimal minification of javascript
- adds CONCATENATE_ASSETS constant to disable concat+min of css and js assets

on show_bug without any extensions installed, this reduces the number of http requests for javascript files from 16 to 4.

note: js files loaded outside of the header will not be touched by this code.
Comment 2 User image Frédéric Buclin 2014-09-10 07:25:06 PDT
I guess that's the wrong patch...
Comment 3 User image Byron Jones ‹:glob› 2014-09-10 09:05:22 PDT
Created attachment 8487261 [details] [diff] [review]
1064395_1.patch

(attaching the correct patch this time)

- concatenates starting_js_urls and javascript_urls into individual requests
- performs minimal minification of javascript
- adds CONCATENATE_ASSETS constant to disable concat+min of css and js assets

on show_bug without any extensions installed, this reduces the number of http requests for javascript files from 16 to 4.

note: js files loaded outside of the header will not be touched by this code.
Comment 4 User image David Lawrence [:dkl] 2014-09-12 09:29:09 PDT
Comment on attachment 8487261 [details] [diff] [review]
1064395_1.patch

Review of attachment 8487261 [details] [diff] [review]:
-----------------------------------------------------------------

Otherwise works well. Quick r+ on next revision if all checks out.

::: Bugzilla/Template.pm
@@ +545,5 @@
>  
> +sub _concatenate_js {
> +    return @_ unless CONCATENATE_ASSETS;
> +    my @sources = map { @$_ } @_;
> +    return unless @sources;

This fails if the $_[0] is not a list such as when accessing index.cgi and no YUI files are loaded. Best to write this way:

+sub _concatenate_js {
+    return @_ unless CONCATENATE_ASSETS;
+    my ($sources) = @_;
+    return [] if !$sources;
+    $sources = ref $sources ? $sources : [$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 ];
+}
Comment 5 User image Byron Jones ‹:glob› 2014-09-17 01:15:26 PDT
Created attachment 8490620 [details] [diff] [review]
1064395_2.patch
Comment 6 User image David Lawrence [:dkl] 2014-09-17 07:09:03 PDT
Comment on attachment 8490620 [details] [diff] [review]
1064395_2.patch

Review of attachment 8490620 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Comment 7 User image Byron Jones ‹:glob› 2014-09-17 09:34:06 PDT
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   2288b46..1bead98  master -> master

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   d4a04eb..c8c2d8e  master -> master
Comment 8 User image Frédéric Buclin 2015-02-21 12:01:02 PST
Added to relnotes for 5.0rc1.

Note You need to log in before you can comment on or make changes to this bug.