r93284 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r93283‎ | r93284 | r93285 >
Date:16:43, 27 July 2011
Author:diebuche
Status:reverted (Comments)
Tags:
Comment:
Fix Bug 11270 & Bug 11555 : Make editsection link more understandable by positioning it directly left ( RTL: right ) of the title.
Patch by Aryeh Gregor, updated by Roan Kattouw, and updated again by me. I also fixed one bug with modern.css.
Tested in IE6,7,8, Chrome & FF in all skins and both LTR and RTL contexts. I tested with floating images above and below the headers and couldn't find regressions.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)
  • /trunk/phase3/skins/chick/IE60Fixes.css (modified) (history)
  • /trunk/phase3/skins/chick/main.css (modified) (history)
  • /trunk/phase3/skins/cologneblue/screen.css (modified) (history)
  • /trunk/phase3/skins/common/oldshared.css (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)
  • /trunk/phase3/skins/common/wikistandard.css (modified) (history)
  • /trunk/phase3/skins/modern/main.css (modified) (history)
  • /trunk/phase3/skins/monobook/main.css (modified) (history)
  • /trunk/phase3/skins/simple/main.css (modified) (history)
  • /trunk/phase3/skins/vector/screen.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/modern/main.css
——@@ -334,12 +334,17 @@
335335 font-size: small;
336336 }
337337
338 -h1, h2 {
 338+.mw-h1,
 339+.mw-h2,
 340+.mw-h3,
 341+.mw-h4,
 342+.mw-h5,
 343+.mw-h6 {
 344+ overflow: hidden;
339345 border-bottom: solid 1px #036;
340346 }
341 -
342 -h1, h2, h3, h4, h5, h6 {
343 - overflow: hidden;
 347+.mw-h3, .mw-h4, .mw-h5, .mw-h6 {
 348+ border-bottom: none;
344349 }
345350
346351 #preftoc {
Index: trunk/phase3/skins/common/wikistandard.css
——@@ -96,18 +96,10 @@
9797 font-size: 150%;
9898 }
9999
100 -h1.pagetitle .editsection {
101 - font-size: 66.7%;
102 -}
103 -
104100 h2 {
105101 font-size: 120%;
106102 }
107103
108 -h2 .editsection {
109 - font-size: 83.3%;
110 -}
111 -
112104 h2, h3, h4, h5, h6 {
113105 margin-bottom: 0;
114106 }
——@@ -116,34 +108,18 @@
117109 font-size: 106.25%;
118110 }
119111
120 -h3 .editsection {
121 - font-size: 94.1%;
122 -}
123 -
124112 h4 {
125113 font-size: 103.125%;
126114 }
127115
128 -h4 .editsection {
129 - font-size: 97.0%;
130 -}
131 -
132116 h5 {
133117 font-size: 100%;
134118 }
135119
136 -h5 .editsection {
137 - font-size: 100%;
138 -}
139 -
140120 h6 {
141121 font-size: 95%;
142122 }
143123
144 -h6 .editsection {
145 - font-size: 105.3%;
146 -}
147 -
148124 hr.sep {
149125 color: gray;
150126 height: 1px;
Index: trunk/phase3/skins/common/oldshared.css
——@@ -4,30 +4,6 @@
55 * CologneBlue, the old pre-Monobook skins
66 */
77
8 -/* For clarity, explicitly state some recommendations from <http://www.w3.org/
9 - TR/CSS21/sample.html> to make sure the editsection links scale right */
10 -
11 -h1 { font-size: 2em; }
12 -h2 { font-size: 1.5em; }
13 -h3 { font-size: 1.17em; }
14 -h5 { font-size: .83em; }
15 -h6 { font-size: .75em; }
16 -h1, h2, h3, h4, h5, h6 {
17 - font-weight: bolder;
18 -}
19 -
20 -/* Now the custom parts */
21 -
22 -/* Make edit sections (which are inside h# tags) normal-sized */
23 -.editsection {
24 - font-weight: normal;
25 -}
26 -h1 .editsection { font-size: 50%; }
27 -h2 .editsection { font-size: 66.7%; }
28 -h3 .editsection { font-size: 85.5%; }
29 -h5 .editsection { font-size: 120%; }
30 -h6 .editsection { font-size: 133%; }
31 -
328 #footer { clear: both }
339 /* images */
3410 /* @noflip */
Index: trunk/phase3/skins/common/shared.css
——@@ -82,14 +82,22 @@
8383 background-color: #fcfcfc;
8484 }
8585
86 -/* Edit section links */
 86+/* Display headings on the same line as edit link */
 87+h1, h2, h3, h4, h5, h6 { display: inline; margin: 0; }
 88+/* But then we have to reintroduce the margin. We use the W3 recommended mar-
 89+ * gins <http://www.w3.org/TR/CSS21/sample.html>, multiplying by the recom-
 90+ * mended font-size. */
 91+.mw-h1 { margin: 1.34em 0; }
 92+.mw-h2 { margin: 1.13em 0; }
 93+.mw-h3 { margin: 0.97em 0; }
 94+.mw-h4 { margin: 1.12em 0; }
 95+.mw-h5 { margin: 1.25em 0; }
 96+.mw-h6 { margin: 1.25em 0; }
 97+
 98+/* Give edi section link some space */
8799 .editsection {
88 - float: right;
89 - margin-left: 5px;
 100+ margin: 0 .8em;
90101 }
91 -/* Correct directionality when page dir is different from site/user dir */
92 -/* @noflip */.mw-content-ltr .editsection, .mw-content-rtl .mw-content-ltr .editsection { float: right; }
93 -/* @noflip */.mw-content-rtl .editsection, .mw-content-ltr .mw-content-rtl .editsection { float: left; }
94102 /**
95103 * File histories
96104 */
Index: trunk/phase3/skins/Vector.php
——@@ -144,7 +144,7 @@
145145 <!-- /sitenotice -->
146146 <?php endif; ?>
147147 <!-- firstHeading -->
148 - <h1 id="firstHeading" class="firstHeading"><?php $this->html( 'title' ) ?></h1>
 148+ <div class="mw-h1 firstHeading"><h1><?php $this->html( 'title' ) ?></h1></div>
149149 <!-- /firstHeading -->
150150 <!-- bodyContent -->
151151 <div id="bodyContent">
Index: trunk/phase3/skins/simple/main.css
——@@ -1,26 +1,3 @@
2 -/* For clarity, explicitly state some recommendations from <http://www.w3.org/
3 - TR/CSS21/sample.html> to make sure the editsection links scale right */
4 -
5 -h1 { font-size: 2em; }
6 -h2 { font-size: 1.5em; }
7 -h3 { font-size: 1.17em; }
8 -h5 { font-size: .83em; }
9 -h6 { font-size: .75em; }
10 -h1, h2, h3, h4, h5, h6 { font-weight: bolder }
11 -
12 -/* Now the custom parts */
13 -
14 -/* Make edit sections (which are inside h# tags) normal-sized */
15 -.editsection {
16 - font-weight: normal;
17 -}
18 -h1 .editsection { font-size: 50% }
19 -h2 .editsection { font-size: 66.7% }
20 -h3 .editsection { font-size: 85.5% }
21 -h5 .editsection { font-size: 120% }
22 -h6 .editsection { font-size: 133% }
23 -
24 -
252 #toolbar {
263 display: none;
274 }
——@@ -86,11 +63,22 @@
8764 }
8865 p img { margin: 0; }
8966
90 -h1, h2, h3, h4, h5, h6 {
91 - margin: 0;
92 - padding-top: 0.5em;
93 - padding-bottom: 0.17em;
 67+.mw-h1, .mw-h2, .mw-h3, .mw-h4, .mw-h5, .mw-h6 {
 68+ margin: 0;
9469 }
 70+/* To replicate the old padding, where h# were block-level elements and edit
 71+ * links were floated, we need to multiply .5em top padding and .17em bottom
 72+ * padding by the header text sizes. We assume the W3 recommendations are
 73+ * used: 200%, 150%, 117%, 100%, 83%, 75% size in that order
 74+ * <http://www.w3.org/TR/CSS21/sample.html>. If not the padding may be margin-
 75+ * ally off. */
 76+.mw-h1 { padding: .94em 0 .32em; } /* (0.5*1.88)em 0 (0.17*1.88)em */
 77+.mw-h2 { padding: .75em 0 .26em; } /* (0.5*1.50)em 0 (0.17*1.50)em */
 78+.mw-h3 { padding: .66em 0 .22em; } /* etc. */
 79+.mw-h4 { padding: .58em 0 .20em; }
 80+.mw-h5 { padding: .50em 0 .17em; }
 81+.mw-h6 { padding: .40em 0 .14em; }
 82+
9583 fieldset {
9684 margin: 1em 0em 1em 0em;
9785 padding: 0em 1em 1em 1em;
——@@ -195,7 +183,7 @@
196184 border: solid 1px black;
197185 }
198186
199 -h1.firstHeading, h2 {
 187+.firstHeading, .mw-h2 {
200188 border-bottom: solid 1px black;
201189 }
202190 #bodyContent a[href ^="http://"],
Index: trunk/phase3/skins/cologneblue/screen.css
——@@ -164,10 +164,6 @@
165165 line-height: 21pt;
166166 }
167167
168 -h1 .editsection {
169 - font-size: 55.6%;
170 -}
171 -
172168 h1.pagetitle {
173169 padding-bottom: 0;
174170 margin-bottom: 0;
——@@ -207,7 +203,7 @@
208204 color: #CC2200;
209205 }
210206
211 -h2, h3, h4, h5, h6 {
 207+.mw-h2, .mw-h3, .mw-h4, .mw-h5, .mw-h6 {
212208 margin-bottom: 0;
213209 }
214210
Index: trunk/phase3/skins/vector/screen.css
——@@ -727,42 +727,41 @@
728728 }
729729
730730 /* Structural Elements */
731 -h1,
732 -h2,
733 -h3,
734 -h4,
735 -h5,
736 -h6 {
 731+.mw-h1,
 732+.mw-h2,
 733+.mw-h3,
 734+.mw-h4,
 735+.mw-h5,
 736+.mw-h6 {
737737 color: black;
738738 background: none;
739 - font-weight: normal;
740739 margin: 0;
741740 overflow: hidden;
742 - padding-top: .5em;
743 - padding-bottom: .17em;
744741 border-bottom: 1px solid #aaa;
745742 width: auto;
746743 }
747 -h1 { font-size: 188%; }
748 -h1 .editsection { font-size: 53%; }
749 -h2 { font-size: 150%; }
750 -h2 .editsection { font-size: 67%; }
751 -h3,
752 -h4,
753 -h5,
754 -h6 {
 744+.mw-h3, .mw-h4, .mw-h5, .mw-h6 {
755745 border-bottom: none;
756 - font-weight: bold;
757746 }
 747+h1, h2 { font-weight: normal; }
 748+h1 { font-size: 188%; }
 749+h2 { font-size: 150%; }
758750 h3 { font-size: 132%; }
759 -h3 .editsection { font-size: 76%; font-weight: normal; }
760751 h4 { font-size: 116%; }
761 -h4 .editsection { font-size: 86%; font-weight: normal; }
762752 h5 { font-size: 100%; }
763 -h5 .editsection { font-weight: normal; }
764753 h6 { font-size: 80%; }
765 -h6 .editsection { font-size: 125%; font-weight: normal; }
766 -.editsection { float: right; }
 754+
 755+/* To replicate the old padding, where h# were block-level elements and edit
 756+ * links were floated, we need to multiply .5em top padding and .17em bottom
 757+ * padding by the header text sizes. */
 758+.mw-h1 { padding: .94em 0 .32em; } /* (0.5*1.88)em 0 (0.17*1.88)em */
 759+.mw-h2 { padding: .75em 0 .26em; } /* (0.5*1.50)em 0 (0.17*1.50)em */
 760+.mw-h3 { padding: .66em 0 .22em; } /* etc. */
 761+.mw-h4 { padding: .58em 0 .20em; }
 762+.mw-h5 { padding: .50em 0 .17em; }
 763+.mw-h6 { padding: .40em 0 .14em; }
 764+
 765+
767766 p {
768767 margin: .4em 0 .5em 0;
769768 line-height: 1.5em;
Index: trunk/phase3/skins/MonoBook.php
——@@ -78,7 +78,7 @@
7979 <a id="top"></a>
8080 <?php if($this->data['sitenotice']) { ?><div id="siteNotice"><?php $this->html('sitenotice') ?></div><?php } ?>
8181
82 - <h1 id="firstHeading" class="firstHeading"><?php $this->html('title') ?></h1>
 82+ <div class="mw-h1 firstHeading"><h1><?php $this->html('title') ?></h1></div>
8383 <div id="bodyContent">
8484 <div id="siteSub"><?php $this->msg('tagline') ?></div>
8585 <div id="contentSub"<?php $this->html('userlangattributes') ?>><?php $this->html('subtitle') ?></div>
Index: trunk/phase3/skins/chick/IE60Fixes.css
——@@ -70,7 +70,6 @@
7171 width: 96%;
7272 }
7373
74 -div.editsection,
7574 #catlinks,
7675 div.tright,
7776 div.tleft {
Index: trunk/phase3/skins/chick/main.css
——@@ -42,35 +42,37 @@
4343 margin: 0.2em 0 0.2em 0;
4444 }
4545
46 -h1, h2, h3, h4, h5, h6 {
 46+/* Headers */
 47+.mw-h1, .mw-h2, .mw-h3, .mw-h4, .mw-h5, .mw-h6 {
4748 color: black;
4849 background: none;
49 - font-weight: normal;
5050 margin: 0;
51 - overflow: hidden;
52 - padding-top: 0.5em;
53 - padding-bottom: 0.17em;
54 - border-bottom: 1px solid #aaaaaa;
 51+ border-bottom: 1px solid #aaa;
5552 }
56 -.editsection {
 53+.mw-h3, .mw-h4, .mw-h5, .mw-h6 {
 54+ border-bottom: none;
 55+}
 56+h1, h2 {
5757 font-weight: normal;
5858 }
5959 h1 { font-size: 188%; }
60 -h1 .editsection { font-size: 53.2%; }
6160 h2 { font-size: 150%; }
62 -h2 .editsection { font-size: 66.7%; }
63 -h3, h4, h5, h6 {
64 - border-bottom: none;
65 - font-weight: bold;
66 -}
6761 h3 { font-size: 132%; }
68 -h3 .editsection { font-size: 75.8%; }
6962 h4 { font-size: 116%; }
70 -h4 .editsection { font-size: 86.2%; }
7163 h5 { font-size: 100%; }
7264 h6 { font-size: 80%; }
73 -h6 .editsection { font-size: 125%; }
7465
 66+/* To replicate the old padding, where h# were block-level elements and edit
 67+ * links were floated, we need to multiply .5em top padding and .17em bottom
 68+ * padding by the header text sizes. */
 69+.mw-h1 { padding: .94em 0 .32em; } /* (0.5*1.88)em 0 (0.17*1.88)em */
 70+.mw-h2 { padding: .75em 0 .26em; } /* (0.5*1.50)em 0 (0.17*1.50)em */
 71+.mw-h3 { padding: .66em 0 .22em; } /* etc. */
 72+.mw-h4 { padding: .58em 0 .20em; }
 73+.mw-h5 { padding: .50em 0 .17em; }
 74+.mw-h6 { padding: .40em 0 .14em; }
 75+
 76+
7577 ul {
7678 line-height: 1.5em;
7779 margin: 0.3em 0 0 1.5em;
Index: trunk/phase3/skins/monobook/main.css
——@@ -107,33 +107,41 @@
108108 margin: .2em 0 .2em 0;
109109 }
110110
111 -h1, h2, h3, h4, h5, h6 {
 111+/* Headers */
 112+.mw-h1, .mw-h2, .mw-h3, .mw-h4, .mw-h5, .mw-h6 {
112113 color: black;
113114 background: none;
114 - font-weight: normal;
115115 margin: 0;
116116 overflow: hidden;
117 - padding-top: .5em;
118 - padding-bottom: .17em;
119117 border-bottom: 1px solid #aaa;
120118 }
 119+.mw-h3, .mw-h4, .mw-h5, .mw-h6 {
 120+ border-bottom: none;
 121+}
 122+h1, h2 { font-weight: normal; }
 123+
121124 h1 { font-size: 188%; }
122 -h1 .editsection { font-size: 53%; }
123125 h2 { font-size: 150%; }
124 -h2 .editsection { font-size: 67%; }
125126 h3, h4, h5, h6 {
126127 border-bottom: none;
127128 font-weight: bold;
128129 }
129130 h3 { font-size: 132%; }
130 -h3 .editsection { font-size: 76%; font-weight: normal; }
131131 h4 { font-size: 116%; }
132 -h4 .editsection { font-size: 86%; font-weight: normal; }
133132 h5 { font-size: 100%; }
134 -h5 .editsection { font-weight: normal; }
135133 h6 { font-size: 80%; }
136 -h6 .editsection { font-size: 125%; font-weight: normal; }
137134
 135+/* To replicate the old padding, where h# were block-level elements and edit
 136+ * links were floated, we need to multiply .5em top padding and .17em bottom
 137+ * padding by the header text sizes. */
 138+.mw-h1 { padding: .94em 0 .32em; } /* (0.5*1.88)em 0 (0.17*1.88)em */
 139+.mw-h2 { padding: .75em 0 .26em; } /* (0.5*1.50)em 0 (0.17*1.50)em */
 140+.mw-h3 { padding: .66em 0 .22em; } /* etc. */
 141+.mw-h4 { padding: .58em 0 .20em; }
 142+.mw-h5 { padding: .50em 0 .17em; }
 143+.mw-h6 { padding: .40em 0 .14em; }
 144+
 145+
138146 ul {
139147 line-height: 1.5em;
140148 list-style-type: square;
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1617,7 +1617,7 @@
16181618 * to ensure that client-side caches do not keep obsolete copies of global
16191619 * styles.
16201620 */
1621 -$wgStyleVersion = '303';
 1621+$wgStyleVersion = '304';
16221622
16231623 /**
16241624 * This will cache static pages for non-logged-in users to reduce
Index: trunk/phase3/includes/Linker.php
——@@ -1506,10 +1506,11 @@
15071507 * @return String: HTML headline
15081508 */
15091509 public static function makeHeadline( $level, $attribs, $anchor, $text, $link, $legacyAnchor = false ) {
1510 - $ret = "<h$level$attribs"
1511 - . $link
 1510+ $ret = "<div class=\"mw-h$level\">"
 1511+ . "<h$level$attribs"
15121512 . " <span class=\"mw-headline\" id=\"$anchor\">$text</span>"
1513 - . "</h$level>";
 1513+ . "</h$level>"
 1514+ . "$link </div>";
15141515 if ( $legacyAnchor !== false ) {
15151516 $ret = "<div id=\"$legacyAnchor\"></div>$ret";
15161517 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r93285r93284 : Update rightClickEdit.jsdiebuche16:56, 27 July 2011
r93299revert r93284 and follow up r93285...hashar19:24, 27 July 2011
r93386Redo r93284 ( Make editsection link more understandable by positioning it dir...diebuche12:26, 28 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r28517* (bug 11555) Make editsection links come after section seadlines. This has n...leon16:28, 15 December 2007
r52410(bug 11270) Put the [edit] link after the section header instead of before, a...catrope20:00, 25 June 2009

Comments

#Comment by Siebrand (talk | contribs)   17:10, 27 July 2011

This needs release notes, as it may upset a lot of custom styling.

#Comment by Krinkle (talk | contribs)   20:13, 27 July 2011

Adding classes to all headings mediawiki outputs and making headings into meaningless divs is quite a significant change. One that I think will never happen. What does that solve ? Anything about semantics ?

#Comment by Brion VIBBER (talk | contribs)   20:21, 27 July 2011

What's actually wrong with this? When it was last committed (r52410) and reverted (r52414) by Roan in 2009, only "unresolved issues" were mentioned.

I asked Roan quickly offline and he's not sure offhand exactly what was wrong then.

#Comment by Krinkle (talk | contribs)   20:44, 27 July 2011

Just quickly ran over this with Roan and Brion over lunch. The easiest solution is probably to just move the editsection from before h2 > span.mw-headline to after it. so the resulting code is: 

<h#>
  <span class="mw-headline">Foobar</span>
  <span class="editsection">[edit]</span>
</h#>
#Comment by DieBuche (talk | contribs)   22:37, 27 July 2011

You seem to be right.There are multiple claims in the bugs that "Any solution that leaves [edit] inside the header isn't a full solution anyway;". The reason stated was that, [edit] would be read by screenreaders differently ( or not at all ? ) if not inside the header.

I can't confirm that with some limited tests; edit is read no matter where exactly in the DOM it's positioned. The only ( probably negligible ) difference is, when you switch the screenreader into a "summary mode", where only the outline of the document is read.

Status & tagging log