Closed Bug 956190 Opened 10 years ago Closed 10 years ago

Merge some of the CSS files (17 is way too much)

Categories

(Bugzilla :: Bugzilla-General, enhancement)

4.5.1
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 5.0

People

(Reporter: LpSolit, Assigned: LpSolit)

Details

(Keywords: perf)

Attachments

(5 files)

We currently have *17* CSS files in skins/standard/ which means 17 files for the web browser to fetch to display all Bugzilla pages. If you use a skin which is not Classic, this means *34* CSS files that the browser will try to fetch. Firebug shows a very clear delay when the number of files to fetch is large, see attachment 701235 [details] from bug 829709. We really need to decrease the number of CSS files as it doesn't make sense to have a whole separate file with just a few CSS rules in it.

global.css should stay as it's called by all templates by default. index.css and page.css should be merged with it.

All admin pages already call admin.css, which should stay. editusers.css and params.css should be merged with it.

About buglists, buglist.css should stay. duplicates.css, reports.css, search_form.css, show_multiple.css and summarize-time.css should be merged with it.

About bugs themselves, attachment.css, dependency-tree.css, enter_bug.css and show_bug.css should be merged, maybe into something like bugs.css.

IE-fixes.css should stay as a separate file as it's only loaded by IE.


This way, we would have only 4 CSS files instead of 16 (+ IE-fixes.css).
This also means less files that Bugzilla needs to look at to get the modification time to pass to the web browser to know if it needs to update its cache!
Sounds good to me.  We could take it a step further if we wanted and dynamically combine them (a lot of webapps do this nowadays).  Compile them all into one file for each page at runtime, hopefully cached in some way.
Attached patch global.css, v1Splinter Review
I'm going to create one patch per merge, else it's quickly going to become a mess to review. First patch: index.css and page.css are merged with global.css.
Assignee: general → LpSolit
Status: NEW → ASSIGNED
Attachment #8383415 - Flags: review?(justdave)
Attachment #8383415 - Flags: review?(justdave) → review+
(In reply to Dave Miller [:justdave] (justdave@bugzilla.org) from comment #2)
> Sounds good to me.  We could take it a step further if we wanted and
> dynamically combine them (a lot of webapps do this nowadays).  Compile them
> all into one file for each page at runtime, hopefully cached in some way.

i've investigated doing that, and have some ideas.  bug 977969
Target Milestone: --- → Bugzilla 5.0
I would like to commit patches one by one. That's easier for me to not omit a file removal or a file rename.
Flags: approval?
Attached patch admin.css, v1Splinter Review
There was some duplicated CSS rules in editusers.css which I managed to remove to use standard rules already defined in admin.css. For instance, using id="admin_table_edit" gives editusers.cgi the same UI as other admin pages.
Attachment #8383588 - Flags: review?(justdave)
Earlier this week, it has been decided that IE7 and older would no longer be supported, so we can kill IE-fixes.css as it was conditionally loaded only if IE version was less or equal to 7.
Attachment #8383596 - Flags: review?(justdave)
(In reply to Frédéric Buclin from comment #5)
> I would like to commit patches one by one. That's easier for me to not omit
> a file removal or a file rename.

works for me.
Comment on attachment 8383415 [details] [diff] [review]
global.css, v1

a+ on this one.
Merge index.css and page.css with global.css:

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified skins/contrib/Dusk/global.css
deleted skins/contrib/Dusk/index.css
modified skins/standard/global.css
deleted skins/standard/index.css
deleted skins/standard/page.css
modified template/en/default/index.html.tmpl
modified template/en/default/welcome-admin.html.tmpl
modified template/en/default/pages/fields.html.tmpl
modified template/en/default/pages/quicksearch.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl
modified template/en/default/pages/release-notes3.html.tmpl
Committed revision 8940.
Comment on attachment 8383588 [details] [diff] [review]
admin.css, v1

r+ and a+ on this one.
Attachment #8383588 - Flags: review?(justdave) → review+
Merge params.css and editusers.css with admin.css:

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified skins/standard/admin.css
deleted skins/standard/editusers.css
deleted skins/standard/params.css
modified template/en/default/admin/params/editparams.html.tmpl
modified template/en/default/admin/users/confirm-delete.html.tmpl
modified template/en/default/admin/users/create.html.tmpl
modified template/en/default/admin/users/edit.html.tmpl
modified template/en/default/admin/users/list.html.tmpl
modified template/en/default/admin/users/search.html.tmpl
Committed revision 8946.
Comment on attachment 8383596 [details] [diff] [review]
IE-fixes.css removal, v1

So basically, by committing this one, we're saying we no longer care if Bugzilla looks good in IE 7 or older, right?

I can get behind that.  Microsoft no longer supports it and it's not safe to use anyway.  Someone using it has bigger problems than whether Bugzilla looks good or not. :-)

r+ / a+
Attachment #8383596 - Flags: review?(justdave) → review+
and that leaves buglist.css and bug.css (or whatever it gets called) left on the to-do for this bug.
Internet Explorer 7 and older are now officially no longer supported.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Template.pm
deleted skins/standard/IE-fixes.css
modified template/en/default/global/header.html.tmpl
Committed revision 8948.
Attached patch buglist.css, v1Splinter Review
Attachment #8388107 - Flags: review?(justdave)
Comment on attachment 8388107 [details] [diff] [review]
buglist.css, v1

Sneaky moving the focus from javascript into html5 attributes as part of this, but I'll take it.

r+/a+

One more to go.
Attachment #8388107 - Flags: review?(justdave) → review+
Merge duplicates.css, reports.css, search_form.css, show_multiple.css and summarize-time.css with buglist.css:

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified skins/standard/buglist.css
deleted skins/standard/duplicates.css
modified skins/standard/global.css
deleted skins/standard/reports.css
deleted skins/standard/search_form.css
deleted skins/standard/show_multiple.css
deleted skins/standard/summarize-time.css
modified template/en/default/bug/show-multiple.html.tmpl
modified template/en/default/bug/summarize-time.html.tmpl
modified template/en/default/reports/components.html.tmpl
modified template/en/default/reports/create-chart.html.tmpl
modified template/en/default/reports/duplicates-simple.html.tmpl
modified template/en/default/reports/duplicates.html.tmpl
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/reports/menu.html.tmpl
modified template/en/default/reports/old-charts.html.tmpl
modified template/en/default/reports/report.html.tmpl
modified template/en/default/search/field.html.tmpl
modified template/en/default/search/form.html.tmpl
modified template/en/default/search/search-advanced.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
modified template/en/default/search/search-report-graph.html.tmpl
modified template/en/default/search/search-report-table.html.tmpl
modified template/en/default/search/search-specific.html.tmpl
Committed revision 8953.
Attached patch bug.css, v1Splinter Review
Final patch. Also removes obsolete entries from .bzrignore.
Attachment #8388138 - Flags: review?(justdave)
Comment on attachment 8388138 [details] [diff] [review]
bug.css, v1

r+/a+
Attachment #8388138 - Flags: review?(justdave) → review+
And that was the last one, so...
Flags: approval? → approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified .bzrignore
deleted skins/standard/attachment.css
added skins/standard/bug.css
deleted skins/standard/dependency-tree.css
deleted skins/standard/enter_bug.css
deleted skins/standard/show_bug.css
modified template/en/default/attachment/create.html.tmpl
modified template/en/default/attachment/diff-header.html.tmpl
modified template/en/default/attachment/edit.html.tmpl
modified template/en/default/attachment/show-multiple.html.tmpl
modified template/en/default/bug/dependency-tree.html.tmpl
modified template/en/default/bug/show-header.html.tmpl
modified template/en/default/bug/create/create-guided.html.tmpl
modified template/en/default/bug/create/create.html.tmpl
Committed revision 8954.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Added to relnotes for 5.0rc1.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: