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

VERIFIED FIXED in 0.9

Status

VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: jason.barnabe, Assigned: paulc)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: sumo_only theme)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
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?

Updated

10 years ago
Blocks: 444274

Comment 1

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

Comment 2

10 years ago
Taking since I did this with AMO already.
Assignee: jason.barnabe → bkrausz

Comment 3

10 years ago
Created attachment 329008 [details]
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.
Attachment #329008 - Flags: review?(laura)

Updated

10 years ago
Target Milestone: --- → 0.6.2

Updated

10 years ago
Assignee: bkrausz → lorchard

Updated

10 years ago
Attachment #329008 - Flags: review?(laura)

Comment 4

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

Updated

10 years ago
Target Milestone: 0.6.2 → 0.6.3

Updated

10 years ago
Status: NEW → ASSIGNED

Comment 5

10 years ago
We can do more here, but pushing to 0.7 for now.
Target Milestone: 0.6.3 → 0.7

Updated

10 years ago
Status: ASSIGNED → NEW
Created attachment 338937 [details] [diff] [review]
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.

Updated

10 years ago
Target Milestone: 0.7 → 0.8

Updated

10 years ago
Target Milestone: 0.8 → 0.9

Updated

10 years ago
Target Milestone: 0.9 → 0.8.2

Comment 7

10 years ago
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.
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

10 years ago
Ok, assigning to Paulc.  Paul, if you have any questions check in with Les.
Assignee: lorchard → paul.craciunoiu
(Assignee)

Comment 10

10 years ago
Created attachment 358464 [details] [diff] [review]
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
Attachment #329008 - Attachment is obsolete: true
Attachment #338937 - Attachment is obsolete: true
Attachment #358464 - Flags: review?(laura)

Comment 11

10 years ago
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
Attachment #358464 - Flags: review?(lorchard)
Attachment #358464 - Flags: review?(laura)
Attachment #358464 - Flags: review+

Comment 12

10 years ago
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".
Target Milestone: 0.8.2 → 0.9
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.
Attachment #358464 - Flags: review?(lorchard) → review-
(Assignee)

Comment 14

10 years ago
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.
(Assignee)

Comment 15

10 years ago
Created attachment 362842 [details] [diff] [review]
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! :)
Attachment #358464 - Attachment is obsolete: true
Attachment #362842 - Flags: review?(laura)
(Assignee)

Updated

10 years ago
Target Milestone: 0.9 → 1.0

Updated

10 years ago
Attachment #362842 - Flags: review?(lorchard)
Attachment #362842 - Flags: review?(laura)
Attachment #362842 - Flags: review+
(Assignee)

Comment 16

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

Comment 17

10 years ago
Paul: yes, assuming Stephen has time to QA.  Commit away.
I can test this today, so as Laura says, please commit away!
(Assignee)

Comment 19

10 years ago
r22617 / r22618 + r22620
QA time Stephen! Thanks!
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Attachment #362842 - Flags: review?(lorchard) → review+
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+
(Assignee)

Comment 21

10 years ago
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.
YSlow reports:

C	1. Make fewer HTTP requests

This page has 14 CSS background images.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
(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
(Assignee)

Comment 25

10 years ago
I'll leave this open to remind myself to add all the CSS files to minify.php.
Summary: Reduce the number of HTTP requests on sumo → Reduce the number of HTTP requests to external JS and CSS files on sumo, and move JS files to the footer
(Assignee)

Comment 26

10 years ago
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!
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: 1.0 → 0.9
Verified FIXED; see the screenshot in comment 27.
Status: RESOLVED → VERIFIED

Updated

9 years ago
Whiteboard: sumo_only theme
You need to log in before you can comment on or make changes to this bug.