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

Status: RESOLVED FIXED

Description

[Frédéric Buclin]

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).

Comment 1

[Frédéric Buclin]

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!

Comment 2

[Dave Miller [:justdave] ([e-mail])]

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.

Comment 3

[Frédéric Buclin]

Created attachment 8383415 [details] [diff] [review]
global.css, v1

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.

Comment 4

[Byron Jones ‹:glob›]

(In reply to Dave Miller [:justdave] ([e-mail]) 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

Comment 5

[Frédéric Buclin]

I would like to commit patches one by one. That's easier for me to not omit a file removal or a file rename.

Comment 6

[Frédéric Buclin]

Created attachment 8383588 [details] [diff] [review]
admin.css, v1

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.

Comment 7

[Frédéric Buclin]

Created attachment 8383596 [details] [diff] [review]
IE-fixes.css removal, v1

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.

Comment 8

[Dave Miller [:justdave] ([e-mail])]

(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 9

[Dave Miller [:justdave] ([e-mail])]

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

a+ on this one.

Comment 10

[Frédéric Buclin]

Merge index.css and page.css with global.css:

Committing to: bzr+ssh://[e-mail]/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 11

[Dave Miller [:justdave] ([e-mail])]

Comment on attachment 8383588 [details] [diff] [review]
admin.css, v1

r+ and a+ on this one.

Comment 12

[Frédéric Buclin]

Merge params.css and editusers.css with admin.css:

Committing to: bzr+ssh://[e-mail]/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 13

[Dave Miller [:justdave] ([e-mail])]

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+

Comment 14

[Dave Miller [:justdave] ([e-mail])]

and that leaves buglist.css and bug.css (or whatever it gets called) left on the to-do for this bug.

Comment 15

[Frédéric Buclin]

Internet Explorer 7 and older are now officially no longer supported.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Template.pm
deleted skins/standard/IE-fixes.css
modified template/en/default/global/header.html.tmpl
Committed revision 8948.

Comment 16

[Frédéric Buclin]

Created attachment 8388107 [details] [diff] [review]
buglist.css, v1

Comment 17

[Dave Miller [:justdave] ([e-mail])]

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.

Comment 18

[Frédéric Buclin]

Merge duplicates.css, reports.css, search_form.css, show_multiple.css and summarize-time.css with buglist.css:

Committing to: bzr+ssh://[e-mail]/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.

Comment 19

[Frédéric Buclin]

Created attachment 8388138 [details] [diff] [review]
bug.css, v1

Final patch. Also removes obsolete entries from .bzrignore.

Comment 20

[Dave Miller [:justdave] ([e-mail])]

Comment on attachment 8388138 [details] [diff] [review]
bug.css, v1

r+/a+

Comment 21

[Dave Miller [:justdave] ([e-mail])]

And that was the last one, so...

Comment 22

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/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.

Comment 23

[Frédéric Buclin]

Added to relnotes for 5.0rc1.