r93261 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r93260‎ | r93261 | r93262 >
Date:13:01, 27 July 2011
Author:diebuche
Status:reverted (Comments)
Tags:
Comment:
Save css optimizations for modern css ( margin: 0 0 0 0 to margin: 0 etc.)
Modified paths:
  • /trunk/phase3/skins/modern/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/modern/main.css
——@@ -1,6 +1,6 @@
22 body {
3 - margin: 0 0 0 0;
4 - padding: 0 0 0 0;
 3+ margin: 0;
 4+ padding: 0;
55 /*font-size: 10pt;*/
66 font-size: x-small;
77
——@@ -24,19 +24,19 @@
2525 position: absolute;
2626 top: 0;
2727 left: 0;
28 - margin: 0 0 0 0;
29 - padding: 0 0em 0 0em;
 28+ margin: 0;
 29+ padding: 0;
3030 border: none;
3131 height: 2em;
3232 width: 100%;
3333
34 - background-color: #003366;
 34+ background-color: #036;
3535 color: white;
3636 }
3737
3838 #mw_header h1 {
39 - margin: 0 0 0 0.5em;
40 - padding: 0 0 0 0;
 39+ margin: 0 0 0 .5em;
 40+ padding: 0;
4141 text-decoration: none;
4242 font-size: 150%;
4343 }
——@@ -46,15 +46,15 @@
4747 top: 2em;
4848 left: 0;
4949 height: 1.5em;
50 - margin: 0 0 0 0;
51 - padding: 0 0 0 0;
 50+ margin: 0;
 51+ padding: 0;
5252 width: 100%;
5353
5454 }
5555
5656 #p-personal div.pBody {
57 - margin: 0 0 0 0;
58 - padding: 0 0 0 0;
 57+ margin: 0;
 58+ padding: 0;
5959 height: 1.5em;
6060 font-variant: small-caps;
6161 }
——@@ -64,8 +64,8 @@
6565 }
6666
6767 #p-personal ul {
68 - margin: 0 0 0 0;
69 - padding: 0 0 0 0;
 68+ margin: 0;
 69+ padding: 0;
7070 display: block;
7171 height: 1.5em;
7272 background-color: #3c78b5;
——@@ -75,7 +75,7 @@
7676 display: block;
7777 float: left;
7878 height: 1.5em;
79 - margin: 0 0 0 0;
 79+ margin: 0;
8080 vertical-align: middle;
8181
8282 font-weight: bold;
——@@ -85,7 +85,7 @@
8686 #p-personal li a {
8787 text-decoration: none;
8888 color: white;
89 - padding: 0 1em 0 1em;
 89+ padding: 0 1em;
9090 }
9191
9292 #p-personal li a:hover {
——@@ -94,7 +94,7 @@
9595 }
9696
9797 #p-personal li:hover {
98 - background-color: #003366;
 98+ background-color: #036;
9999 }
100100
101101 #jump-to-nav {
——@@ -111,40 +111,40 @@
112112 margin: 0 0 0 14em;
113113
114114 background-color: white;
115 - border-top: solid 1px #bbbbbb;
116 - border-left: solid 1px #bbbbbb;
117 - border-bottom: solid 1px #bbbbbb;
 115+ border-top: solid 1px #bbb;
 116+ border-left: solid 1px #bbb;
 117+ border-bottom: solid 1px #bbb;
118118
119119 line-height: 1.5em;
120 - padding: 0 1em 1em 1em;
 120+ padding: 0 1em 1em;
121121 }
122122
123123 #mw_portlets {
124124 width: 14em;
125125
126 - border-right: solid 1px #bbbbbb;
 126+ border-right: solid 1px #bbb;
127127 background-color: #f0f0f0;
128128 }
129129
130130 #mw_main {
131 - padding: 0 0 0 0;
132 - margin: 0 0 0 0;
 131+ padding: 0;
 132+ margin: 0;
133133 margin-top: 3.5em;
134134 }
135135
136136 div.mw_clear {
137 - margin: 0 0 0 0;
138 - padding: 0 0 0 0;
 137+ margin: 0;
 138+ padding: 0;
139139 clear: both;
140140 }
141141
142142 .portlet {
143 - padding: 0 0 0 0;
144 - margin: 0 0 0 0;
 143+ padding: 0;
 144+ margin: 0;
145145 }
146146
147147 .portlet div.pBody {
148 - padding: 0em 0 0.5em 0;
 148+ padding: 0 0 .5em;
149149 }
150150
151151 textarea {
——@@ -167,9 +167,9 @@
168168 }
169169
170170 .portlet h5 {
171 - padding: 0.1em 0 0.3em 1em;
172 - margin: 0 0 0 0;
173 - background-color: #dddddd;
 171+ padding: .1em 0 .3em 1em;
 172+ margin: 0;
 173+ background-color: #ddd;
174174 font-weight: bold;
175175 border-bottom: solid 1px #3c78b5;
176176 height: 1.1em;
——@@ -177,7 +177,7 @@
178178
179179 .portlet ul {
180180 margin: 0 0 0 1.5em;
181 - padding: 0 0 0 0;
 181+ padding: 0;
182182 }
183183
184184 #mw_portlets .portlet ul {
——@@ -191,13 +191,13 @@
192192
193193 #p-cactions {
194194 height: 1.5em;
195 - padding: 0 0 0 0;
 195+ padding: 0;
196196 margin: 0 0 0 14em;
197197 }
198198
199199 #p-cactions div.pBody {
200 - margin: 0 0 0 0;
201 - padding: 0 0 0 0;
 200+ margin: 0;
 201+ padding: 0;
202202 }
203203
204204 #p-cactions a,
——@@ -208,13 +208,13 @@
209209
210210 #p-cactions ul {
211211 display: inline;
212 - margin: 0 0 0 0;
213 - padding: 0 0 0 0;
 212+ margin: 0;
 213+ padding: 0;
214214 }
215215
216216 #p-cactions li {
217 - margin: 0 0.5em 0 0.5em;
218 - padding: 0 0.2em 0 0.2em;
 217+ margin: 0 .5em;
 218+ padding: 0 .2em;
219219 display: block;
220220 float: left;
221221 height: 1.5em;
——@@ -222,14 +222,14 @@
223223 }
224224
225225 #p-cactions li.selected {
226 - background-color: #bbbbbb;
 226+ background-color: #bbb;
227227 }
228228
229229 #p-cactions li a,
230230 #p-cactions li a:hover,
231231 #p-cactions li a:visited {
232232 text-decoration: underline;
233 - color: #003366;
 233+ color: #036;
234234 }
235235
236236 #p-cactions li.selected a,
——@@ -251,15 +251,15 @@
252252 background-color: #f0f0f0;
253253 /* @embed */
254254 background: url(footer-grad.png) repeat-x 0 0;
255 - padding: 10px 1em 1em 1em;
 255+ padding: 10px 1em 1em;
256256 clear:both;
257 - color: #444444;
 257+ color: #444;
258258 }
259259
260260 #footer a,
261261 #footer a:hover,
262262 #footer a:visited {
263 - color: #444444;
 263+ color: #444;
264264 text-decoration: underline;
265265 }
266266
——@@ -270,13 +270,13 @@
271271 #footer li {
272272 display: inline;
273273 list-style-type: none;
274 - padding: 0 0 0 0;
275 - margin: 0 0 0 0;
 274+ padding: 0;
 275+ margin: 0;
276276 }
277277
278278 #footer ul {
279 - padding: 0 0 0 0;
280 - margin: 0 0 0 0;
 279+ padding: 0;
 280+ margin: 0;
281281 }
282282
283283 div.printfooter {
——@@ -284,7 +284,7 @@
285285 }
286286
287287 p {
288 - margin: 1em 0 1em 0;
 288+ margin: 1em 0;
289289 }
290290
291291 hr {
——@@ -292,7 +292,7 @@
293293 color: #aaa;
294294 background-color: #aaa;
295295 border: 0;
296 - margin: .2em 0 .2em 0;
 296+ margin: .2em 0;
297297 }
298298
299299 #contentSub {
——@@ -302,13 +302,13 @@
303303 }
304304
305305 #mw_portlets form {
306 - margin: 0 0 0 0;
307 - padding: 0 0 0 0;
 306+ margin: 0;
 307+ padding: 0;
308308 }
309309
310310 a {
311311 text-decoration: none;
312 - color: #003366;
 312+ color: #036;
313313 background: none;
314314 }
315315 a:visited {
——@@ -335,7 +335,7 @@
336336 }
337337
338338 h1, h2 {
339 - border-bottom: solid 1px #003366;
 339+ border-bottom: solid 1px #036;
340340 }
341341
342342 h1, h2, h3, h4, h5, h6 {
——@@ -344,15 +344,15 @@
345345
346346 #preftoc {
347347 width: 100%;
348 - margin: 0 0 0 0;
349 - padding: 0 0 0 0;
 348+ margin: 0;
 349+ padding: 0;
350350 height: 1.5em;
351351 clear: right;
352352 }
353353
354354 #preftoc li {
355 - margin: 0 0.5em 0 0.5em;
356 - padding: 0 0.2em 0 0.2em;
 355+ margin: 0 .5em;
 356+ padding: 0 .2em;
357357 display: block;
358358 float: left;
359359 height: 1.5em;
——@@ -360,9 +360,9 @@
361361 }
362362
363363 #preferences {
364 - margin: 0 0 0 0;
365 - padding: 0em 1em 1em 1em;
366 - border: solid 1px #bbbbbb;
 364+ margin: 0;
 365+ padding: 0 1em 1em;
 366+ border: solid 1px #bbb;
367367 clear: left; /* Multi-line toc should not push data to horizontally */
368368 }
369369
——@@ -389,14 +389,14 @@
390390 }
391391
392392 #preftoc li.selected {
393 - background-color: #bbbbbb;
 393+ background-color: #bbb;
394394 }
395395
396396 #preftoc li a,
397397 #preftoc li a:hover,
398398 #preftoc li a:visited {
399399 text-decoration: underline;
400 - color: #003366;
 400+ color: #036;
401401 }
402402
403403 #preftoc li.selected a,
——@@ -538,7 +538,7 @@
539539 margin: .5em 1.4em .8em 0;
540540 }
541541 img.thumbborder {
542 - border: 1px solid #dddddd;
 542+ border: 1px solid #ddd;
543543 }
544544 .mw-warning {
545545 border: 1px solid #aaa;
——@@ -549,22 +549,22 @@
550550
551551 #toc,
552552 .toc {
553 - margin: 0 0 0 0;
554 - padding: 0 0 0 0;
 553+ margin: 0;
 554+ padding: 0;
555555 border-spacing: 0;
556556 background-color: #f0f0f0;
557 - border: solid 1px #bbbbbb;
 557+ border: solid 1px #bbb;
558558 }
559559
560560 #toc tr, #toc td {
561 - margin: 0 0 0 0;
562 - padding: 0 0 0 0;
 561+ margin: 0;
 562+ padding: 0;
563563 }
564564
565565 #toctitle {
566566 border-bottom: solid 1px #3c78b5;
567 - background-color: #dddddd;
568 - margin: 0 0 0 0;
 567+ background-color: #ddd;
 568+ margin: 0;
569569 }
570570
571571 #toc h2,
——@@ -586,7 +586,7 @@
587587 .toc ul {
588588 list-style-type: none;
589589 list-style-image: none;
590 - margin: 0 1em 0 1em;
 590+ margin: 0 1em;
591591 padding-left: 0;
592592 text-align: left;
593593 }
——@@ -608,10 +608,10 @@
609609 }
610610
611611 .catlinks {
612 - border: solid 1px #bbbbbb;
 612+ border: solid 1px #bbb;
613613 background-color: #f0f0f0;
614 - padding: 0.1em 0.3em 0.1em 0.3em;
615 - margin: 0 0 0 0;
 614+ padding: .1em .3em;
 615+ margin: 0;
616616 }
617617
618618 #mw_header h1,
——@@ -665,9 +665,9 @@
666666
667667 .editExternally {
668668 border: 1px solid gray;
669 - background-color: #ffffff;
 669+ background-color: #FFF;
670670 padding: 3px;
671 - margin-top: 0.5em;
 671+ margin-top: .5em;
672672 float: left;
673673 font-size: small;
674674 text-align: center;
——@@ -692,7 +692,7 @@
693693
694694 /* @bug 1714 */
695695 input#wpSave, input#wpDiff {
696 - margin-right: 0.33em;
 696+ margin-right: .33em;
697697 }
698698
699699 #wpSave {
——@@ -733,7 +733,7 @@
734734 div#searchTargetHide {
735735 float: right;
736736 border: solid 1px black;
737 - background: gainsboro;
 737+ background: #DCDCDC;
738738 padding: 2px;
739739 }
740740
——@@ -761,7 +761,7 @@
762762 .templatesUsed { margin-top: 1.5em; }
763763
764764 .mw-summary-preview {
765 - margin: 0.1em 0;
 765+ margin: .1em 0;
766766 }
767767
768768 /* Friendlier slave lag warnings */
——@@ -772,18 +772,18 @@
773773 margin: 3px auto;
774774 }
775775 div.mw-lag-warn-normal {
776 - border: 1px solid #FFCC66;
777 - background-color: #FFFFCC;
 776+ border: 1px solid #FC6;
 777+ background-color: #FFC;
778778 }
779779 div.mw-lag-warn-high {
780780 font-weight: bold;
781 - border: 2px solid #FF0033;
782 - background-color: #FFCCCC;
 781+ border: 2px solid #F03;
 782+ background-color: #FCC;
783783 }
784784
785785 .MediaTransformError {
786786 background-color: #ccc;
787 - padding: 0.1em;
 787+ padding: .1em;
788788 }
789789 .MediaTransformError td {
790790 text-align: center;
——@@ -817,7 +817,7 @@
818818 }
819819
820820 #p-cactions li.new a {
821 - color: #cc2200;
 821+ color: #c20;
822822 }
823823
824824 span.subpages {
——@@ -827,7 +827,7 @@
828828
829829 pre {
830830 border: solid 1px #3c78b5;
831 - padding: 0.4em;
 831+ padding: .4em;
832832 background-color: #f0f0f0;
833833 }
834834
——@@ -837,29 +837,29 @@
838838
839839 .mw-topboxes {
840840 border-collapse: collapse;
841 - margin: 0 -1em 1em -1em;
842 - padding: 0 0 8px 0;
 841+ margin: 0 -1em 1em;
 842+ padding: 0 0 8px;
843843 /* @embed */
844844 background: url(footer-grad.png) repeat-x bottom left;
845845 }
846846
847847 .mw-topbox p {
848 - padding: 0 0 0 0;
849 - margin: 0 0 0 0;
 848+ padding: 0;
 849+ margin: 0;
850850 }
851851
852852 .mw-topbox {
853853 color: black;
854854 font-weight: bold;
855 - margin: 0 0 0 0;
856 - padding: 0 1em 0 1em;
 855+ margin: 0;
 856+ padding: 0 1em;
857857 vertical-align: middle;
858858 border-collapse: collapse;
859 - border-bottom: solid 1px #bbbbbb;
 859+ border-bottom: solid 1px #bbb;
860860 }
861861
862862 #siteSub {
863 - background-color: #dddddd;
 863+ background-color: #ddd;
864864 }
865865
866866 /* emulate center */
——@@ -874,11 +874,11 @@
875875
876876 /* table standards */
877877 .toccolours {
878 - border: 1px solid #bbbbbb;
 878+ border: 1px solid #bbb;
879879 background-color: #f0f0f0;
880 - border-spacing: 0pt;
881 - margin: 0pt;
882 - padding: 0pt;
 880+ border-spacing: 0;
 881+ margin: 0;
 882+ padding: 0;
883883 }
884884
885885 /* Tooltips are outside of the normal body code, so this helps make the size of the text sensible */

Follow-up revisions

RevisionCommit summaryAuthorDate
r95407Revert most of r93261...reedy18:09, 24 August 2011

Comments

#Comment by Siebrand (talk | contribs)   13:04, 27 July 2011

I'm not a CSS expert, but reading "modern" in your commit message, does this possibly impact how older (still supported) browser versions will display pages? If so, CSS should not (yet) be optimised.

#Comment by DieBuche (talk | contribs)   13:32, 27 July 2011

Stuff like margin: 0; has been valid since CSS 1 ( Quote from http://www.w3.org/TR/CSS1/#margin: If four length values are specified they apply to top, right, bottom and left respectively. If there is only one value, it applies to all sides, if there are two or three, the missing values are taken from the opposite side. ) and this very basic stuff is implemented in all browsers who claim at least CSS1 support.

#Comment by Platonides (talk | contribs)   13:58, 27 July 2011

What's the point of this change?

You save 472 bytes on the uncompressed file, 62 when gzipped.

It doesn't seem worth doing this. I don't care about "margin: 0 0 0 0;" vs "margin: 0;" but I do find 0.3em and #003366 easier to read than .3em and #036.

If we want this file to be smaller, we should perform a proper minimization or directly serve it via the resourceloader.

#Comment by Nikerabbit (talk | contribs)   14:46, 27 July 2011

I agree.

#Comment by Aaron Schulz (talk | contribs)   22:11, 1 August 2011

I'd have to agree (especially on the color thing).

#Comment by Hashar (talk | contribs)   08:34, 10 August 2011

I do like the following changes: - '0pt' to '0' - '0 0 0 0' and '0 0' to '0' - named color 'gainsboro' to hex #DCDCDC

I do not like:

A) 

- margin: 0 -1em 1em -1em;
+ margin: 0 -1em 1em;

Three parameters? I am sure CSS has this covered but I will probably not remember that the second parameter covers both right and left. The four parameters format is way easier to read.

B) Colors with six digits are easier to copy paste in a drawing software / color picker.

C) '.3em' is barely readable. Keep it as '0.3em' which is obvious.


I appreciate having someone willing to optimize / clean up our years old stylesheets. Still, please keep readability above optimizations hacks. In the long term, the code is easier to maintain and we probably do not care that much about saving a few bytes in CSS.


Marking 'fixme'

Status & tagging log