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)
Tracking
()
RESOLVED
FIXED
Bugzilla 5.0
People
(Reporter: LpSolit, Assigned: LpSolit)
Details
(Keywords: perf)
Attachments
(5 files)
14.37 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
12.44 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
3.62 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
34.02 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
30.61 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•10 years ago
|
||
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•10 years ago
|
||
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.
Assignee | ||
Comment 3•10 years ago
|
||
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.
Updated•10 years ago
|
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
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → Bugzilla 5.0
Assignee | ||
Comment 5•10 years ago
|
||
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?
Assignee | ||
Comment 6•10 years ago
|
||
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)
Assignee | ||
Comment 7•10 years ago
|
||
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)
Comment 8•10 years ago
|
||
(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•10 years ago
|
||
Comment on attachment 8383415 [details] [diff] [review] global.css, v1 a+ on this one.
Assignee | ||
Comment 10•10 years ago
|
||
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 11•10 years ago
|
||
Comment on attachment 8383588 [details] [diff] [review] admin.css, v1 r+ and a+ on this one.
Attachment #8383588 -
Flags: review?(justdave) → review+
Assignee | ||
Comment 12•10 years ago
|
||
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 13•10 years ago
|
||
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+
Comment 14•10 years ago
|
||
and that leaves buglist.css and bug.css (or whatever it gets called) left on the to-do for this bug.
Assignee | ||
Comment 15•10 years ago
|
||
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.
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8388107 -
Flags: review?(justdave)
Comment 17•10 years ago
|
||
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+
Assignee | ||
Comment 18•10 years ago
|
||
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.
Assignee | ||
Comment 19•10 years ago
|
||
Final patch. Also removes obsolete entries from .bzrignore.
Attachment #8388138 -
Flags: review?(justdave)
Comment 20•10 years ago
|
||
Comment on attachment 8388138 [details] [diff] [review] bug.css, v1 r+/a+
Attachment #8388138 -
Flags: review?(justdave) → review+
Assignee | ||
Comment 22•10 years ago
|
||
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
Assignee | ||
Comment 23•9 years ago
|
||
Added to relnotes for 5.0rc1.
You need to log in
before you can comment on or make changes to this bug.
Description
•