r85232 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r85231‎ | r85232 | r85233 >
Date:07:48, 3 April 2011
Author:nikola
Status:reverted (Comments)
Tags:
Comment:
New hook SkinTemplateLanguageBoxEnd
Modified paths:
  • /branches/nikola/phase3/skins/MonoBook.php (modified) (history)
  • /branches/nikola/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: branches/nikola/phase3/skins/MonoBook.php
——@@ -268,7 +268,10 @@
269269 <?php foreach($this->data['language_urls'] as $key => $langlink) { ?>
270270 <?php echo $this->makeListItem($key, $langlink); ?>
271271
272 -<?php } ?>
 272+<?php
 273+ }
 274+ wfRunHooks( 'SkinTemplateLanguageBoxEnd', array( &$this ) );
 275+?>
273276 </ul>
274277 </div>
275278 </div>
Index: branches/nikola/phase3/skins/Vector.php
——@@ -263,7 +263,7 @@
264264 break;
265265 case 'LANGUAGES':
266266 if ( $this->data['language_urls'] ) {
267 - $this->renderPortal("lang", $this->data['language_urls'], "otherlanguages");
 267+ $this->renderPortal("lang", $this->data['language_urls'], "otherlanguages", "SkinTemplateLanguageBoxEnd");
268268 }
269269 break;
270270 default:

Follow-up revisions

RevisionCommit summaryAuthorDate
r85356- rv r85232...nikola18:41, 4 April 2011

Comments

#Comment by Nikerabbit (talk | contribs)   13:50, 3 April 2011

New hooks should be documented.

#Comment by Dantman (talk | contribs)   03:18, 4 April 2011

Noooo......

I've been trying to eliminate these kind of hooks. Echoing markup like this is bad for the skinning system, you should at least try just modifying language_urls in extensions instead.

Hardcoding this into individual skins as well is bad.

#Comment by Nikola Smolenski (talk | contribs)   04:51, 4 April 2011

I guess I could do something like this: 

	function languageBox() {
		if( $this->data['language_urls'] ) {
			wfRunHooks( 'SkinTemplateLanguageBoxBefore', array( &$this ) );

Would that be better? It is still possible to output text, but I could say in documentation that it is preferable to edit language_urls.

I don't think it's possible to do this outside of a skin, because SkinTemplate builds language_urls from pure text passed to it via ParserOutput, and so it is not possible to add action links.

#Comment by Dantman (talk | contribs)   05:22, 4 April 2011

We already have a hook to edit template data like language_urls, SkinTemplateOutputPageBeforeExec.

#Comment by Nikola Smolenski (talk | contribs)   18:43, 4 April 2011

Hey, it actually works :)

I reverted this revision in r85356, however in order to simulate [edit] links I had to make an addition to SkinTemplate::makeListItem() - could you see if it's OK with you?

Status & tagging log