r78017 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78016‎ | r78017 | r78018 >
Date:21:36, 7 December 2010
Author:catrope
Status:ok
Tags:
Comment:
1.17: Back out r77893 (beginning of skin system refactor, committed the day before the branch point). Doing something this major right before a release that already has so many major changes sounds like a bad idea.
Modified paths:
  • /branches/REL1_17/phase3/docs/hooks.txt (modified) (history)
  • /branches/REL1_17/phase3/includes/SkinTemplate.php (modified) (history)
  • /branches/REL1_17/phase3/skins/MonoBook.php (modified) (history)
  • /branches/REL1_17/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: branches/REL1_17/phase3/skins/Vector.php
@@ -339,7 +339,7 @@
340340 * QuickTemplate class for Vector skin
341341 * @ingroup Skins
342342 */
343 -class VectorTemplate extends BaseTemplate {
 343+class VectorTemplate extends QuickTemplate {
344344
345345 /* Members */
346346
@@ -592,13 +592,36 @@
593593 <h5<?php $this->html('userlangattributes') ?>><?php $this->msg( 'toolbox' ) ?></h5>
594594 <div class="body">
595595 <ul>
596 -<?php
597 - foreach ( $this->getToolbox() as $key => $tbitem ): ?>
598 - <?php echo $this->makeListItem($key, $tbitem); ?>
599 -
600 -<?php
601 - endforeach;
602 - wfRunHooks( 'SkinTemplateToolboxEnd', array( &$this ) ); ?>
 596+ <?php if( $this->data['notspecialpage'] ): ?>
 597+ <li id="t-whatlinkshere"><a href="<?php echo htmlspecialchars( $this->data['nav_urls']['whatlinkshere']['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( 't-whatlinkshere' ) ?>><?php $this->msg( 'whatlinkshere' ) ?></a></li>
 598+ <?php if( $this->data['nav_urls']['recentchangeslinked'] ): ?>
 599+ <li id="t-recentchangeslinked"><a href="<?php echo htmlspecialchars( $this->data['nav_urls']['recentchangeslinked']['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( 't-recentchangeslinked' ) ?>><?php $this->msg( 'recentchangeslinked-toolbox' ) ?></a></li>
 600+ <?php endif; ?>
 601+ <?php endif; ?>
 602+ <?php if( isset( $this->data['nav_urls']['trackbacklink'] ) ): ?>
 603+ <li id="t-trackbacklink"><a href="<?php echo htmlspecialchars( $this->data['nav_urls']['trackbacklink']['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( 't-trackbacklink' ) ?>><?php $this->msg( 'trackbacklink' ) ?></a></li>
 604+ <?php endif; ?>
 605+ <?php if( $this->data['feeds']): ?>
 606+ <li id="feedlinks">
 607+ <?php foreach( $this->data['feeds'] as $key => $feed ): ?>
 608+ <a id="<?php echo Sanitizer::escapeId( "feed-$key" ) ?>" href="<?php echo htmlspecialchars( $feed['href'] ) ?>" rel="alternate" type="application/<?php echo $key ?>+xml" class="feedlink"<?php echo $this->skin->tooltipAndAccesskey( 'feed-' . $key ) ?>><?php echo htmlspecialchars( $feed['text'] ) ?></a>
 609+ <?php endforeach; ?>
 610+ </li>
 611+ <?php endif; ?>
 612+ <?php foreach( array( 'contributions', 'log', 'blockip', 'emailuser', 'upload', 'specialpages' ) as $special ): ?>
 613+ <?php if( $this->data['nav_urls'][$special]): ?>
 614+ <li id="t-<?php echo $special ?>"><a href="<?php echo htmlspecialchars( $this->data['nav_urls'][$special]['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( 't-' . $special ) ?>><?php $this->msg( $special ) ?></a></li>
 615+ <?php endif; ?>
 616+ <?php endforeach; ?>
 617+ <?php if( !empty( $this->data['nav_urls']['print']['href'] ) ): ?>
 618+ <li id="t-print"><a href="<?php echo htmlspecialchars( $this->data['nav_urls']['print']['href'] ) ?>" rel="alternate"<?php echo $this->skin->tooltipAndAccesskey( 't-print' ) ?>><?php $this->msg( 'printableversion' ) ?></a></li>
 619+ <?php endif; ?>
 620+ <?php if ( !empty( $this->data['nav_urls']['permalink']['href'] ) ): ?>
 621+ <li id="t-permalink"><a href="<?php echo htmlspecialchars( $this->data['nav_urls']['permalink']['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( 't-permalink' ) ?>><?php $this->msg( 'permalink' ) ?></a></li>
 622+ <?php elseif ( $this->data['nav_urls']['permalink']['href'] === '' ): ?>
 623+ <li id="t-ispermalink"<?php echo $this->skin->tooltip( 't-ispermalink' ) ?>><?php $this->msg( 'permalink' ) ?></li>
 624+ <?php endif; ?>
 625+ <?php wfRunHooks( 'SkinTemplateToolboxEnd', array( &$this ) ); ?>
603626 </ul>
604627 </div>
605628 </div>
@@ -611,9 +634,8 @@
612635 <h5<?php $this->html('userlangattributes') ?>><?php $this->msg( 'otherlanguages' ) ?></h5>
613636 <div class="body">
614637 <ul>
615 - <?php foreach ( $this->data['language_urls'] as $key => $langlink ): ?>
616 - <?php echo $this->makeListItem($key, $langlink); ?>
617 -
 638+ <?php foreach ( $this->data['language_urls'] as $langlink ): ?>
 639+ <li class="<?php echo htmlspecialchars( $langlink['class'] ) ?>"><a href="<?php echo htmlspecialchars( $langlink['href'] ) ?>" title="<?php echo htmlspecialchars( $langlink['title'] ) ?>"><?php echo $langlink['text'] ?></a></li>
618640 <?php endforeach; ?>
619641 </ul>
620642 </div>
@@ -628,9 +650,8 @@
629651 <div class="body">
630652 <?php if ( is_array( $content ) ): ?>
631653 <ul>
632 - <?php foreach( $content as $key => $val ): ?>
633 - <?php echo $this->makeListItem($key, $val); ?>
634 -
 654+ <?php foreach( $content as $val ): ?>
 655+ <li id="<?php echo Sanitizer::escapeId( $val['id'] ) ?>"<?php if ( $val['active'] ): ?> class="active" <?php endif; ?>><a href="<?php echo htmlspecialchars( $val['href'] ) ?>"<?php echo $this->skin->tooltipAndAccesskey( $val['id'] ) ?>><?php echo htmlspecialchars( $val['text'] ) ?></a></li>
635656 <?php endforeach; ?>
636657 </ul>
637658 <?php else: ?>
Index: branches/REL1_17/phase3/skins/MonoBook.php
@@ -51,7 +51,7 @@
5252 * @todo document
5353 * @ingroup Skins
5454 */
55 -class MonoBookTemplate extends BaseTemplate {
 55+class MonoBookTemplate extends QuickTemplate {
5656 var $skin;
5757 /**
5858 * Template filter callback for MonoBook skin.
@@ -269,11 +269,49 @@
270270 <div class="pBody">
271271 <ul>
272272 <?php
273 - foreach ( $this->getToolbox() as $key => $tbitem ) { ?>
274 - <?php echo $this->makeListItem($key, $tbitem); ?>
275 -
 273+ if($this->data['notspecialpage']) { ?>
 274+ <li id="t-whatlinkshere"><a href="<?php
 275+ echo htmlspecialchars($this->data['nav_urls']['whatlinkshere']['href'])
 276+ ?>"<?php echo $this->skin->tooltipAndAccesskey('t-whatlinkshere') ?>><?php $this->msg('whatlinkshere') ?></a></li>
276277 <?php
 278+ if( $this->data['nav_urls']['recentchangeslinked'] ) { ?>
 279+ <li id="t-recentchangeslinked"><a href="<?php
 280+ echo htmlspecialchars($this->data['nav_urls']['recentchangeslinked']['href'])
 281+ ?>"<?php echo $this->skin->tooltipAndAccesskey('t-recentchangeslinked') ?>><?php $this->msg('recentchangeslinked-toolbox') ?></a></li>
 282+<?php }
277283 }
 284+ if( isset( $this->data['nav_urls']['trackbacklink'] ) && $this->data['nav_urls']['trackbacklink'] ) { ?>
 285+ <li id="t-trackbacklink"><a href="<?php
 286+ echo htmlspecialchars($this->data['nav_urls']['trackbacklink']['href'])
 287+ ?>"<?php echo $this->skin->tooltipAndAccesskey('t-trackbacklink') ?>><?php $this->msg('trackbacklink') ?></a></li>
 288+<?php }
 289+ if($this->data['feeds']) { ?>
 290+ <li id="feedlinks"><?php foreach($this->data['feeds'] as $key => $feed) {
 291+ ?><a id="<?php echo Sanitizer::escapeId( "feed-$key" ) ?>" href="<?php
 292+ echo htmlspecialchars($feed['href']) ?>" rel="alternate" type="application/<?php echo $key ?>+xml" class="feedlink"<?php echo $this->skin->tooltipAndAccesskey('feed-'.$key) ?>><?php echo htmlspecialchars($feed['text'])?></a>&#160;
 293+ <?php } ?></li><?php
 294+ }
 295+
 296+ foreach( array('contributions', 'log', 'blockip', 'emailuser', 'upload', 'specialpages') as $special ) {
 297+
 298+ if($this->data['nav_urls'][$special]) {
 299+ ?><li id="t-<?php echo $special ?>"><a href="<?php echo htmlspecialchars($this->data['nav_urls'][$special]['href'])
 300+ ?>"<?php echo $this->skin->tooltipAndAccesskey('t-'.$special) ?>><?php $this->msg($special) ?></a></li>
 301+<?php }
 302+ }
 303+
 304+ if(!empty($this->data['nav_urls']['print']['href'])) { ?>
 305+ <li id="t-print"><a href="<?php echo htmlspecialchars($this->data['nav_urls']['print']['href'])
 306+ ?>" rel="alternate"<?php echo $this->skin->tooltipAndAccesskey('t-print') ?>><?php $this->msg('printableversion') ?></a></li><?php
 307+ }
 308+
 309+ if(!empty($this->data['nav_urls']['permalink']['href'])) { ?>
 310+ <li id="t-permalink"><a href="<?php echo htmlspecialchars($this->data['nav_urls']['permalink']['href'])
 311+ ?>"<?php echo $this->skin->tooltipAndAccesskey('t-permalink') ?>><?php $this->msg('permalink') ?></a></li><?php
 312+ } elseif ($this->data['nav_urls']['permalink']['href'] === '') { ?>
 313+ <li id="t-ispermalink"<?php echo $this->skin->tooltip('t-ispermalink') ?>><?php $this->msg('permalink') ?></li><?php
 314+ }
 315+
278316 wfRunHooks( 'MonoBookTemplateToolboxEnd', array( &$this ) );
279317 wfRunHooks( 'SkinTemplateToolboxEnd', array( &$this ) );
280318 ?>
@@ -291,9 +329,10 @@
292330 <h5<?php $this->html('userlangattributes') ?>><?php $this->msg('otherlanguages') ?></h5>
293331 <div class="pBody">
294332 <ul>
295 -<?php foreach($this->data['language_urls'] as $key => $langlink) { ?>
296 - <?php echo $this->makeListItem($key, $langlink); ?>
297 -
 333+<?php foreach($this->data['language_urls'] as $langlink) { ?>
 334+ <li class="<?php echo htmlspecialchars($langlink['class'])?>"><?php
 335+ ?><a href="<?php echo htmlspecialchars($langlink['href']) ?>" title="<?php
 336+ echo htmlspecialchars($langlink['title']) ?>"><?php echo $langlink['text'] ?></a></li>
298337 <?php } ?>
299338 </ul>
300339 </div>
@@ -310,9 +349,10 @@
311350 <div class='pBody'>
312351 <?php if ( is_array( $cont ) ) { ?>
313352 <ul>
314 -<?php foreach($cont as $key => $val) { ?>
315 - <?php echo $this->makeListItem($key, $val); ?>
316 -
 353+<?php foreach($cont as $val) { ?>
 354+ <li id="<?php echo Sanitizer::escapeId($val['id']) ?>"<?php
 355+ if ( $val['active'] ) { ?> class="active" <?php }
 356+ ?>><a href="<?php echo htmlspecialchars($val['href']) ?>"<?php echo $this->skin->tooltipAndAccesskey($val['id']) ?>><?php echo htmlspecialchars($val['text']) ?></a></li>
317357 <?php } ?>
318358 </ul>
319359 <?php } else {
Index: branches/REL1_17/phase3/docs/hooks.txt
@@ -1165,15 +1165,6 @@
11661166 "SkinTemplate"-type skins.
11671167 $tools: array of tools
11681168
1169 -'BaseTemplateToolbox': Called by BaseTemplate when building the $toolbox array
1170 -and returning it for the skin to output. You can add items to the toolbox while
1171 -still letting the skin make final decisions on skin-specific markup conventions
1172 -using this hook.
1173 -&$sk: The BaseTemplate base skin template
1174 -&$toolbox: An array of toolbox items, see BaseTemplate::getToolbox and
1175 - BaseTemplate::makeListItem for details on the format of individual
1176 - items inside of this array
1177 -
11781169 'NewRevisionFromEditComplete': called when a revision was inserted
11791170 due to an edit
11801171 $article: the article edited
Index: branches/REL1_17/phase3/includes/SkinTemplate.php
@@ -1183,178 +1183,3 @@
11841184 return ( $msg != '-' ) && ( $msg != '' ); # ????
11851185 }
11861186 }
1187 -
1188 -/**
1189 - * New base template for a skin's template extended from QuickTemplate
1190 - * this class features helper methods that provide common ways of interacting
1191 - * with the data stored in the QuickTemplate
1192 - */
1193 -abstract class BaseTemplate extends QuickTemplate {
1194 -
1195 - /**
1196 - * Create an array of common toolbox items from the data in the quicktemplate
1197 - * stored by SkinTemplate.
1198 - * The resulting array is built acording to a format intended to be passed
1199 - * through makeListItem to generate the html.
1200 - */
1201 - function getToolbox() {
1202 - wfProfileIn( __METHOD__ );
1203 -
1204 - $toolbox = array();
1205 - if ( $this->data['notspecialpage'] ) {
1206 - $toolbox["whatlinkshere"] = $this->data['nav_urls']['whatlinkshere'];
1207 - $toolbox["whatlinkshere"]["id"] = "t-whatlinkshere";
1208 - if ( $this->data['nav_urls']['recentchangeslinked'] ) {
1209 - $toolbox["recentchangeslinked"] = $this->data['nav_urls']['recentchangeslinked'];
1210 - $toolbox["recentchangeslinked"]["msg"] = "recentchangeslinked-toolbox";
1211 - $toolbox["recentchangeslinked"]["id"] = "t-recentchangeslinked";
1212 - }
1213 - }
1214 - if( isset( $this->data['nav_urls']['trackbacklink'] ) && $this->data['nav_urls']['trackbacklink'] ) {
1215 - $toolbox["trackbacklink"] = $this->data['nav_urls']['trackbacklink'];
1216 - $toolbox["trackbacklink"]["id"] = "t-trackbacklink";
1217 - }
1218 - if ( $this->data['feeds'] ) {
1219 - $toolbox["feeds"]["id"] = "feedlinks";
1220 - $toolbox["feeds"]["links"] = array();
1221 - foreach ( $this->data['feeds'] as $key => $feed ) {
1222 - $toolbox["feeds"]["links"][$key] = $feed;
1223 - $toolbox["feeds"]["links"][$key]["id"] = "feed-$key";
1224 - $toolbox["feeds"]["links"][$key]["rel"] = "alternate";
1225 - $toolbox["feeds"]["links"][$key]["type"] = "application/{$key}+xml";
1226 - $toolbox["feeds"]["links"][$key]["class"] = "feedlink";
1227 - }
1228 - }
1229 - foreach ( array('contributions', 'log', 'blockip', 'emailuser', 'upload', 'specialpages') as $special ) {
1230 - if ( $this->data['nav_urls'][$special] ) {
1231 - $toolbox[$special] = $this->data['nav_urls'][$special];
1232 - $toolbox[$special]["id"] = "t-$special";
1233 - }
1234 - }
1235 - if ( !empty( $this->data['nav_urls']['print']['href'] ) ) {
1236 - $toolbox["print"] = $this->data['nav_urls']['print'];
1237 - $toolbox["print"]["rel"] = "alternate";
1238 - $toolbox["print"]["msg"] = "printableversion";
1239 - }
1240 - if ( !empty( $this->data['nav_urls']['permalink']['href'] ) ) {
1241 - $toolbox["permalink"] = $this->data['nav_urls']['permalink'];
1242 - $toolbox["permalink"]["id"] = "t-permalink";
1243 - } elseif ( $this->data['nav_urls']['permalink']['href'] === '' ) {
1244 - $toolbox["permalink"] = $this->data['nav_urls']['permalink'];
1245 - unset( $toolbox["permalink"]["href"] );
1246 - $toolbox["ispermalink"]["tooltiponly"] = true;
1247 - $toolbox["ispermalink"]["id"] = "t-ispermalink";
1248 - $toolbox["ispermalink"]["msg"] = "permalink";
1249 - }
1250 - wfRunHooks( 'BaseTemplateToolbox', array( &$this, &$toolbox ) );
1251 - wfProfileOut( __METHOD__ );
1252 - return $toolbox;
1253 - }
1254 -
1255 - /**
1256 - * Makes a link, usually used by makeListItem to generate a link for an item
1257 - * in a list used in navigation lists, portlets, portals, sidebars, etc...
1258 - *
1259 - * $key is a string, usually a key from the list you are generating this link from
1260 - * $item is an array containing some of a specific set of keys.
1261 - * The text of the link will be generated either from the contents of the "text"
1262 - * key in the $item array, if a "msg" key is present a message by that name will
1263 - * be used, and if neither of those are set the $key will be used as a message name.
1264 - * If a "href" key is not present makeLink will just output htmlescaped text.
1265 - * The href, id, class, rel, and type keys are used as attributes for the link if present.
1266 - * If an "id" or "single-id" (if you don't want the actual id to be output on the link)
1267 - * is present it will be used to generate a tooltip and accesskey for the link.
1268 - * If you don't want an accesskey set $item["tooltiponly"] = true;
1269 - */
1270 - function makeLink($key, $item) {
1271 - if ( isset($item["text"]) ) {
1272 - $text = $item["text"];
1273 - } else {
1274 - $text = $this->translator->translate( isset($item["msg"]) ? $item["msg"] : $key );
1275 - }
1276 -
1277 - if ( !isset($item["href"]) ) {
1278 - return htmlspecialchars($text);
1279 - }
1280 -
1281 - $attrs = array();
1282 - foreach ( array("href", "id", "class", "rel", "type") as $attr ) {
1283 - if ( isset($item[$attr]) ) {
1284 - $attrs[$attr] = $item[$attr];
1285 - }
1286 - }
1287 -
1288 - if ( isset($item["id"]) ) {
1289 - $item["single-id"] = $item["id"];
1290 - }
1291 - if ( isset($item["single-id"]) ) {
1292 - if ( isset($item["tooltiponly"]) && $item["tooltiponly"] ) {
1293 - $attrs["title"] = $this->skin->titleAttrib($item["single-id"]);
1294 - if ( $attrs["title"] === false ) {
1295 - unset($attrs["title"]);
1296 - }
1297 - } else {
1298 - $attrs = array_merge($attrs, $this->skin->tooltipAndAccesskeyAttribs($item["single-id"]));
1299 - }
1300 - }
1301 -
1302 - return Html::element( "a", $attrs, $text );
1303 - }
1304 -
1305 - /**
1306 - * Generates a list item for a navigation, portlet, portal, sidebar... etc list
1307 - * $key is a string, usually a key from the list you are generating this link from
1308 - * $item is an array of list item data containing some of a specific set of keys.
1309 - * The "id" and "class" keys will be used as attributes for the list item,
1310 - * if "active" contains a value of true a "active" class will also be appended to class.
1311 - * If you want something other than a <li> you can pass a tag name such as
1312 - * "tag" => "span" in the $options array to change the tag used.
1313 - * link/content data for the list item may come in one of two forms
1314 - * A "links" key may be used, in which case it should contain an array with
1315 - * a list of links to include inside the list item, see makeLink for the format
1316 - * of individual links array items.
1317 - * Otherwise the relevant keys from the list item $item array will be passed
1318 - * to makeLink instead. Note however that "id" and "class" are used by the
1319 - * list item directly so they will not be passed to makeLink
1320 - * (however the link will still support a tooltip and accesskey from it)
1321 - * If you need an id or class on a single link you should include a "links"
1322 - * array with just one link item inside of it.
1323 - */
1324 - function makeListItem($key, $item, $options = array()) {
1325 - if ( isset($item["links"]) ) {
1326 - $html = '';
1327 - foreach ( $item["links"] as $linkKey => $link ) {
1328 - $html .= $this->makeLink($linkKey, $link);
1329 - }
1330 - } else {
1331 - $link = array();
1332 - foreach ( array("text", "msg", "href", "rel", "type", "tooltiponly") as $k ) {
1333 - if ( isset($item[$k]) ) {
1334 - $link[$k] = $item[$k];
1335 - }
1336 - }
1337 - if ( isset($item["id"]) ) {
1338 - // The id goes on the <li> not on the <a> for single links
1339 - // but makeSidebarLink still needs to know what id to use when
1340 - // generating tooltips and accesskeys.
1341 - $link["single-id"] = $item["id"];
1342 - }
1343 - $html = $this->makeLink($key, $link);
1344 - }
1345 -
1346 - $attrs = array();
1347 - foreach ( array("id", "class") as $attr ) {
1348 - if ( isset($item[$attr]) ) {
1349 - $attrs[$attr] = $item[$attr];
1350 - }
1351 - }
1352 - if ( isset($item["active"]) && $item["active"] ) {
1353 - $attrs["class"] .= " active";
1354 - $attrs["class"] = trim($attrs["class"]);
1355 - }
1356 - return Html::rawElement( isset($options["tag"]) ? $options["tag"] : "li", $attrs, $html );
1357 - }
1358 -
1359 -
1360 -}
1361 -

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r77893More skin system improvements; Create a new BaseTemplate class extended from ...dantman17:47, 6 December 2010

Status & tagging log