444265 - Reduce the number of HTTP requests to external JS and CSS files on sumo, and move JS files to the footer

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect)

Description

[Jason Barnabe (np)]

http://support.mozilla.com/tiki-view_forum.php?locale=en-US&forumId=1 loads 6 JavaScript files and 4 stylesheets, not to mention files loaded indirectly. I think this can be cut down.

-I don't think cb.js is used any more.
-freetags.css can probably be crammed somewhere else
-Do we still want to use Urchin?

Comment 1

[David Tenser [:djst]]

Yes to using Urchin until we have everything covered in Omniture. Can you look into the css files though?

Comment 2

[Brian Krausz]

Taking since I did this with AMO already.

Comment 3

[Brian Krausz]

Attachment: First round of improvements

Added yui.css and yui.js to main css and js file, respectively.  Looked into freetags, seems totally useless, commented out addition command.

Still need a build process to shrink down these files, etc, but that can be added on top.

Comment 4

[Brian Krausz]

Seems I already checked this in, it was a minor change, didn't need a review anyway.

Comment 5

[Laura Thomson :laura]

We can do more here, but pushing to 0.7 for now.

Comment 6

[Les Orchard [:lorchard]]

Attachment: patch to introduce usage of minify.php for CSS and JS

This patch is not quite ready for review or check-in, but it has been sitting unchanged on my hard drive for a few weeks.  I'm attaching it here to at least snapshot the progress.

Comment 7

[Laura Thomson :laura]

Les, do you have time to work on this?  Paulc is available to help.  If you don't have time, he can take it over.

Comment 8

[Les Orchard [:lorchard]]

I'm pretty well tied up with some AMO stats changes this week.  So, if someone else wants to pick up that patch and carry it forward, that would be great.  It probably needs some dusting off since it's a few months old.

Comment 9

[Laura Thomson :laura]

Ok, assigning to Paulc.  Paul, if you have any questions check in with Les.

Comment 10

[Paul Craciunoiu [:paulc]]

Attachment: Updated Les' patch

Attached patch that updates Les' patch with the new paths (we moved stuff to webroot) and also moves minify into webroot/lib/minify

Tested locally, seems to work fine.
After this I'll look into what other .tpl files might have been missed that could also make use of minify.php

Comment 11

[Laura Thomson :laura]

Comment on attachment 358464 [details] [diff] [review]
Updated Les' patch

Asking lorchard for second review, WFM.

The other thing we'll need to do is document the build process, similar to what is done here but for SUMO: https://wiki.mozilla.org/Update:Build_Process

Comment 12

[Laura Thomson :laura]

Also, since we're changing our build process, let's hold this for 0.9, as 0.8.2 is supposed to a be "mini release".

Comment 13

[Les Orchard [:lorchard]]

Comment on attachment 358464 [details] [diff] [review]
Updated Les' patch

Looks like this is breaking the CSS a bit, at least on the front page.  Haven't had a chance to dig too deeply yet, but I wonder if things are applied out of order once minified.

Also, with respect to the AMO build process, this Minify package doesn't need a build process.  It minifies CSS and JS on the fly and stashes the results in memcache.  I think I did it that way because it looks like there are lots of permutations of JS / CSS on any given SUMO page in different situations, and this approach can capture and cache them dynamically.

Comment 14

[Paul Craciunoiu [:paulc]]

I'll try to see why the CSS is breaking on the front page. If you notice any other pages that it is broken on, add them here.

The patch seemed to work fine for me, locally, after some toying around and refresh. But since it could potentially break CSS on the entire site, we should be very careful.

Comment 15

[Paul Craciunoiu [:paulc]]

Attachment: Updated updated Les' patch :)

New patch.
1. So, as Les said, the previous patch broke some css, but that was because the stylesheet URL was passed with an additional '/' at the beginning, making the link end up like "css__styles_mozms" which wasn't right.
2. CSS Imports weren't working, because CSS requires imports to be at the beginning of the file, and so we needed a way to make all imports come before ALL files... I added that in Minify.php, but not for all cases, just for the mass case. If we want to build this into Minify and contribute, I could do that, but this works for our purposes now.

I tested the most popular pages, all looks fine. Yay! :)

Comment 16

[Paul Craciunoiu [:paulc]]

Can this bug make it into 0.9 if I commit the patch today?

Comment 17

[Laura Thomson :laura]

Paul: yes, assuming Stephen has time to QA.  Commit away.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

I can test this today, so as Laura says, please commit away!

Comment 19

[Paul Craciunoiu [:paulc]]

r22617 / r22618 + r22620
QA time Stephen! Thanks!

Comment 20

[Les Orchard [:lorchard]]

Comment on attachment 362842 [details] [diff] [review]
Updated updated Les' patch :)

I was just starting to check this out when I saw it was checked in.  But, belated though I am, r+

Comment 21

[Paul Craciunoiu [:paulc]]

Oh yeah, I should mention I added in a few lines to also move imports to the beginning of the file for separate preferences. However, that branch of code is never run for our site.

Anyway, if you know how to, Les, we can contribute that specific piece of code to the Minify library, if it's possible.

Comment 22

[Stephen Donner [:stephend] Not actively reading bugmail]

YSlow reports:

C	1. Make fewer HTTP requests

This page has 14 CSS background images.

Comment 23

[Les Orchard [:lorchard]]

Hmm.  Sprite-ifying those background images will be a job in itself.  

I'd suggest renaming this bug to be specific to minifying the CSS and JS and
sticking the JS in the footer in order to address #6, #10, and most of #1 for
bug 444274.

Then, file another dependent for bug 444274 to track combining all the images
into a master sprite image.

Comment 24

[Les Orchard [:lorchard]]

(In reply to comment #21)

> Anyway, if you know how to, Les, we can contribute that specific piece of code
> to the Minify library, if it's possible.

I think I originally got the Minify library from here:

http://code.google.com/p/minify/

But, I didn't check much past making sure the license was BSD.  Might be worth working up a patch to mail back to the authors.  That could be a good bug to file

Comment 25

[Paul Craciunoiu [:paulc]]

I'll leave this open to remind myself to add all the CSS files to minify.php.

Comment 26

[Paul Craciunoiu [:paulc]]

r22655 / r22656
Added the rest of CSS and JS files. I searched for all the .js and .css files, so the list should be complete.

Les: I'll try and talk to the guys that did the Minify library too. Thanks!

Comment 27

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot

Yay!

Comment 28

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; see the screenshot in comment 27.