1064395 - concatenate and slightly minify javascript files

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[:glob ✱]

similar to what we do for css, we should concatenate and slightly minify javascript files.

Comment 1

[:glob ✱]

Attachment: 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

[Frédéric Buclin]

I guess that's the wrong patch...

Comment 3

[:glob ✱]

Attachment: 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

[David Lawrence [:dkl]]

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

[:glob ✱]

Attachment: 1064395_2.patch

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 8490620 [details] [diff] [review]
1064395_2.patch

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

r=dkl

Comment 7

[:glob ✱]

To ssh://[e-mail]/bugzilla/bugzilla.git
   2288b46..1bead98  master -> master

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   d4a04eb..c8c2d8e  master -> master

Comment 8

[Frédéric Buclin]

Added to relnotes for 5.0rc1.