r97657 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97656‎ | r97657 | r97658 >
Date:19:47, 20 September 2011
Author:robin
Status:ok (Comments)
Tags:
Comment:
Fix support of legacy skins Standard & CologneBlue for user language direction: the topbar and footer used a "quickbar compensator" which relied on the topbar and footer following the content language direction.
Therefore it's easier to do it via CSS (which simplifies PHP code).
See also bug 31031.
Modified paths:
  • /trunk/phase3/includes/SkinLegacy.php (modified) (history)
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/CologneBlue.php
——@@ -32,19 +32,23 @@
3333 if ( 2 == $qb ) { # Right
3434 $rules[] = "/* @noflip */#quickbar { position: absolute; right: 4px; }";
3535 $rules[] = "/* @noflip */#article { margin-left: 4px; margin-right: 148px; }";
 36+ $rules[] = "/* @noflip */#topbar, #footer { margin-right: 152px; }";
3637 } elseif ( 1 == $qb ) {
3738 $rules[] = "/* @noflip */#quickbar { position: absolute; left: 4px; }";
3839 $rules[] = "/* @noflip */#article { margin-left: 148px; margin-right: 4px; }";
 40+ $rules[] = "/* @noflip */#topbar, #footer { margin-left: 152px; }";
3941 } elseif ( 3 == $qb ) { # Floating left
4042 $rules[] = "/* @noflip */#quickbar { position:absolute; left:4px }";
4143 $rules[] = "/* @noflip */#topbar { margin-left: 148px }";
4244 $rules[] = "/* @noflip */#article { margin-left:148px; margin-right: 4px; }";
4345 $rules[] = "/* @noflip */body>#quickbar { position:fixed; left:4px; top:4px; overflow:auto; bottom:4px;}"; # Hides from IE
 46+ $rules[] = "/* @noflip */#topbar, #footer { margin-left: 152px; }";
4447 } elseif ( 4 == $qb ) { # Floating right
4548 $rules[] = "/* @noflip */#quickbar { position: fixed; right: 4px; }";
4649 $rules[] = "/* @noflip */#topbar { margin-right: 148px }";
4750 $rules[] = "/* @noflip */#article { margin-right: 148px; margin-left: 4px; }";
4851 $rules[] = "/* @noflip */body>#quickbar { position: fixed; right: 4px; top: 4px; overflow: auto; bottom:4px;}"; # Hides from IE
 52+ $rules[] = "/* @noflip */#topbar, #footer { margin-left: 152px; }";
4953 }
5054 $style = implode( "\n", $rules );
5155 $out->addInlineStyle( $style, 'flip' );
——@@ -105,10 +109,6 @@
106110 $s .= "\n<div id='footer'>";
107111 $s .= '<table width="98%" border="0" cellspacing="0"><tr>';
108112
109 - $qb = $this->getSkin()->qbSetting();
110 - if ( 1 == $qb || 3 == $qb ) { # Left
111 - $s .= $this->getQuickbarCompensator();
112 - }
113113 $s .= '<td class="bottom">';
114114
115115 $s .= $this->bottomLinks();
——@@ -127,12 +127,9 @@
128128 $s .= "\n<br />" . $this->pageStats();
129129
130130 $s .= '</td>';
131 - if ( 2 == $qb ) { # Right
132 - $s .= $this->getQuickbarCompensator();
133 - }
134131 $s .= "</tr></table>\n</div>\n</div>\n";
135132
136 - if ( 0 != $qb ) {
 133+ if ( $this->getSkin()->qbSetting() != 0 ) {
137134 $s .= $this->quickBar();
138135 }
139136 return $s;
Index: trunk/phase3/skins/Standard.php
——@@ -31,9 +31,11 @@
3232 if ( 2 == $qb ) { # Right
3333 $rules[] = "/* @noflip */#quickbar { position: absolute; top: 4px; right: 4px; border-left: 2px solid #000000; }";
3434 $rules[] = "/* @noflip */#article, #mw-data-after-content { margin-left: 4px; margin-right: 152px; }";
 35+ $rules[] = "/* @noflip */#topbar, #footer { margin-right: 152px; }";
3536 } elseif ( 1 == $qb || 3 == $qb ) {
3637 $rules[] = "/* @noflip */#quickbar { position: absolute; top: 4px; left: 4px; border-right: 1px solid gray; }";
3738 $rules[] = "/* @noflip */#article, #mw-data-after-content { margin-left: 152px; margin-right: 4px; }";
 39+ $rules[] = "/* @noflip */#topbar, #footer { margin-left: 152px; }";
3840 if( 3 == $qb ) {
3941 $rules[] = "/* @noflip */#quickbar { position: fixed; padding: 4px; }";
4042 }
——@@ -41,6 +43,7 @@
4244 $rules[] = "/* @noflip */#quickbar { position: fixed; right: 0px; top: 0px; padding: 4px;}";
4345 $rules[] = "/* @noflip */#quickbar { border-right: 1px solid gray; }";
4446 $rules[] = "/* @noflip */#article, #mw-data-after-content { margin-right: 152px; margin-left: 4px; }";
 47+ $rules[] = "/* @noflip */#topbar, #footer { margin-right: 152px; }";
4548 }
4649 $style = implode( "\n", $rules );
4750 $out->addInlineStyle( $style, 'flip' );
——@@ -54,7 +57,6 @@
5558 * @return string
5659 */
5760 function doAfterContent() {
58 - global $wgContLang;
5961 wfProfileIn( __METHOD__ );
6062 wfProfileIn( __METHOD__ . '-1' );
6163
——@@ -64,17 +66,7 @@
6567
6668 wfProfileOut( __METHOD__ . '-1' );
6769 wfProfileIn( __METHOD__ . '-2' );
68 -
69 - $qb = $this->getSkin()->qbSetting();
70 - $shove = ( $qb != 0 );
71 - $left = ( $qb == 1 || $qb == 3 );
72 -
73 - if ( $shove && $left ) { # Left
74 - $s .= $this->getQuickbarCompensator();
75 - }
76 - wfProfileOut( __METHOD__ . '-2' );
77 - wfProfileIn( __METHOD__ . '-3' );
78 - $l = $wgContLang->alignStart();
 70+ $l = $this->getSkin()->getLang()->alignStart();
7971 $s .= "<td class='bottom' align='$l' valign='top'>";
8072
8173 $s .= $this->bottomLinks();
——@@ -86,17 +78,14 @@
8779 . '<br /><span id="pagestats">' . $this->pageStats() . '</span>';
8880
8981 $s .= '</td>';
90 - if ( $shove && !$left ) { # Right
91 - $s .= $this->getQuickbarCompensator();
92 - }
9382 $s .= "</tr></table>\n</div>\n</div>\n";
9483
95 - wfProfileOut( __METHOD__ . '-3' );
96 - wfProfileIn( __METHOD__ . '-4' );
97 - if ( 0 != $qb ) {
 84+ wfProfileOut( __METHOD__ . '-2' );
 85+ wfProfileIn( __METHOD__ . '-3' );
 86+ if ( $this->getSkin()->qbSetting() != 0 ) {
9887 $s .= $this->quickBar();
9988 }
100 - wfProfileOut( __METHOD__ . '-4' );
 89+ wfProfileOut( __METHOD__ . '-3' );
10190 wfProfileOut( __METHOD__ );
10291 return $s;
10392 }
Index: trunk/phase3/includes/SkinLegacy.php
——@@ -90,11 +90,10 @@
9191 }
9292
9393 function doBeforeContent() {
94 - global $wgContLang;
 94+ global $wgLang;
9595 wfProfileIn( __METHOD__ );
9696
9797 $s = '';
98 - $qb = $this->getSkin()->qbSetting();
9998
10099 $langlinks = $this->otherLanguages();
101100 if ( $langlinks ) {
——@@ -107,25 +106,20 @@
108107 }
109108
110109 $s .= "\n<div id='content'>\n<div id='topbar'>\n" .
111 - "<table border='0' cellspacing='0' width='98%'>\n<tr>\n";
 110+ "<table border='0' cellspacing='0' width='100%'>\n<tr>\n";
112111
113 - $shove = ( $qb != 0 );
114 - $left = ( $qb == 1 || $qb == 3 );
115 -
116 - if ( !$shove ) {
 112+ if ( $this->getSkin()->qbSetting() == 0 ) {
117113 $s .= "<td class='top' align='left' valign='top' rowspan='{$rows}'>\n" .
118 - $this->getSkin()->logoText() . '</td>';
119 - } elseif ( $left ) {
120 - $s .= $this->getQuickbarCompensator( $rows );
 114+ $this->getSkin()->logoText( $wgLang->alignStart() ) . '</td>';
121115 }
122116
123 - $l = $wgContLang->alignStart();
 117+ $l = $wgLang->alignStart();
124118 $s .= "<td {$borderhack} align='$l' valign='top'>\n";
125119
126120 $s .= $this->topLinks();
127121 $s .= '<p class="subtitle">' . $this->pageTitleLinks() . "</p>\n";
128122
129 - $r = $wgContLang->alignEnd();
 123+ $r = $wgLang->alignEnd();
130124 $s .= "</td>\n<td {$borderhack} valign='top' align='$r' nowrap='nowrap'>";
131125 $s .= $this->nameAndLogin();
132126 $s .= "\n<br />" . $this->searchForm() . '</td>';
——@@ -134,10 +128,6 @@
135129 $s .= "</tr>\n<tr>\n<td class='top' colspan=\"2\">$langlinks</td>\n";
136130 }
137131
138 - if ( $shove && !$left ) { # Right
139 - $s .= $this->getQuickbarCompensator( $rows );
140 - }
141 -
142132 $s .= "</tr>\n</table>\n</div>\n";
143133 $s .= "\n<div id='article'>\n";
144134
——@@ -593,6 +583,9 @@
594584 return $wgLang->pipeList( $s );
595585 }
596586
 587+ /**
 588+ * @deprecated in 1.19
 589+ */
597590 function getQuickbarCompensator( $rows = 1 ) {
598591 return "<td width='152' rowspan='{$rows}'>&#160;</td>";
599592 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r981521.18wmf1: MFT r93912, r95318, r97403, r97650, r97657, r97661, r97687, r97777catrope18:13, 26 September 2011
r984521.18: MFT r97403, r97614, r97657, r97661reedy21:10, 29 September 2011
r103672Fix an issue with the header of CologneBlue. Introduced in r97657...hartman13:48, 19 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r97622As reported in bug 31031, WebKit can mess up the TOC title (which is in user ...robin13:05, 20 September 2011

Comments

#Comment by SPQRobin (talk | contribs)   19:53, 20 September 2011

It may need merging to 1.18 / 1.18wmf1, but apparently it was broken since it exists: see e.g. http://he.wikipedia.org/wiki/%D7%A2%D7%9E%D7%95%D7%93_%D7%A8%D7%90%D7%A9%D7%99?useskin=standard&uselang=he

Status & tagging log