concatenate and slightly minify javascript files

RESOLVED FIXED in Bugzilla 5.0

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

({bmo-goal})

unspecified
Bugzilla 5.0
bmo-goal
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
similar to what we do for css, we should concatenate and slightly minify javascript files.
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Keywords: bmo-goal
(Assignee)

Comment 1

3 years ago
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.
Attachment #8486976 - Flags: review?(sgreen)

Comment 2

3 years ago
I guess that's the wrong patch...
(Assignee)

Comment 3

3 years ago
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.
Attachment #8486976 - Attachment is obsolete: true
Attachment #8486976 - Flags: review?(sgreen)
Attachment #8487261 - Flags: review?(sgreen)

Updated

3 years ago
Attachment #8487261 - Flags: review?(sgreen) → review?(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 ];
+}
Attachment #8487261 - Flags: review?(dkl) → review-
(Assignee)

Comment 5

3 years ago
Created attachment 8490620 [details] [diff] [review]
1064395_2.patch
Attachment #8487261 - Attachment is obsolete: true
Attachment #8490620 - Flags: review?(dkl)
Comment on attachment 8490620 [details] [diff] [review]
1064395_2.patch

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

r=dkl
Attachment #8490620 - Flags: review?(dkl) → review+
(Assignee)

Updated

3 years ago
Flags: approval+
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 5.0
(Assignee)

Comment 7

3 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Keywords: relnote
Resolution: --- → FIXED

Updated

3 years ago
Blocks: 1072110

Comment 8

2 years ago
Added to relnotes for 5.0rc1.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.