r96408 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96407‎ | r96408 | r96409 >
Date:08:12, 7 September 2011
Author:ialex
Status:deferred (Comments)
Tags:todo 
Comment:
* Use static methods instead of an extension function to create an object and set the hooks
* Always declare $wgHidensNamespaces to avoid register_globals vulnerability
* Fix comment
Modified paths:
  • /trunk/extensions/HideNamespace/HideNamespace.php (modified) (history)

Diff [purge]

Index: trunk/extensions/HideNamespace/HideNamespace.php
@@ -20,7 +20,6 @@
2121 $wgExtensionMessagesFiles['HideNamespace'] = $dir . 'HideNamespace.i18n.php';
2222 $wgExtensionMessagesFiles['HideNamespaceMagic'] = $dir . 'HideNamespace.i18n.magic.php';
2323
24 -$wgExtensionFunctions[] = 'wfHideNamespaceSetup';
2524 $wgExtensionCredits['other'][] = array(
2625 'path' => __FILE__,
2726 'name' => 'HideNamespace',
@@ -30,27 +29,22 @@
3130 'url' => 'http://www.mediawiki.org/wiki/Extension:HideNamespace',
3231 );
3332
34 -function wfHideNamespaceSetup() {
35 - global $wgHooks;
 33+$wgHidensNamespaces = array();
3634
37 - $extHidensObj = new ExtensionHideNamespace;
 35+$wgHooks['ParserFirstCallInit'][] = 'ExtensionHideNamespace::registerParser';
 36+$wgHooks['ArticleViewHeader'][] = 'ExtensionHideNamespace::onArticleViewHeader';
 37+$wgHooks['BeforePageDisplay'][] = 'ExtensionHideNamespace::onBeforePageDisplay';
3838
39 - // Register hooks
40 - $wgHooks['ParserFirstCallInit'][] = array( &$extHidensObj, 'registerParser' );
41 - $wgHooks['ArticleViewHeader'][] = array( &$extHidensObj, 'onArticleViewHeader' );
42 - $wgHooks['BeforePageDisplay'][] = array( &$extHidensObj, 'onBeforePageDisplay' );
43 -}
44 -
4539 class ExtensionHideNamespace {
46 - private $namespace, $namespaceText;
47 - private $hide = null;
 40+ private static $namespaceText;
 41+ private static $hide = null;
4842
4943 /**
5044 * Register the parser functions
5145 */
52 - public function registerParser( $parser ) {
53 - $parser->setFunctionHook( 'hidens', array( &$this, 'hideNs' ) );
54 - $parser->setFunctionHook( 'showns', array( &$this, 'showNs' ) );
 46+ public static function registerParser( $parser ) {
 47+ $parser->setFunctionHook( 'hidens', array( __CLASS__, 'hideNs' ) );
 48+ $parser->setFunctionHook( 'showns', array( __CLASS__, 'showNs' ) );
5549
5650 return true;
5751 }
@@ -58,8 +52,8 @@
5953 /**
6054 * Callback for our parser function {{#hidens:}}
6155 */
62 - public function hideNs() {
63 - $this->hide = true;
 56+ public static function hideNs() {
 57+ self::$hide = true;
6458
6559 return null;
6660 }
@@ -67,8 +61,8 @@
6862 /**
6963 * Callback for our parser function {{#showns:}}
7064 */
71 - public function showNs() {
72 - $this->hide = false;
 65+ public static function showNs() {
 66+ self::$hide = false;
7367
7468 return null;
7569 }
@@ -79,14 +73,14 @@
8074 * Retrieves the namespace and localized namespace text and decides whether the
8175 * namespace should be hidden
8276 */
83 - public function onArticleViewHeader( $article ) {
 77+ public static function onArticleViewHeader( $article ) {
8478 global $wgHidensNamespaces, $wgContLang;
8579
86 - $this->namespace = $article->getTitle()->getNamespace();
87 - $this->namespaceText = $wgContLang->getNsText( $this->namespace );
 80+ $namespace = $article->getTitle()->getNamespace();
 81+ self::$namespaceText = $wgContLang->getNsText( $namespace );
8882
89 - if( $this->namespace == NS_MAIN ) {
90 - $this->hide = false;
 83+ if( $namespace == NS_MAIN ) {
 84+ self::$hide = false;
9185 } else {
9286 /**
9387 * Hide namespace if either
@@ -94,26 +88,25 @@
9589 * - the current namespace is in $wgHidensNamespaces AND
9690 * {{#showns:}} wasn't called
9791 */
98 - $visibilityForced = !is_null( $this->hide );
99 - $hideByUser = $visibilityForced && $this->hide;
100 - $hideBySetting = isset( $wgHidensNamespaces ) &&
101 - in_array( $this->namespace, $wgHidensNamespaces );
 92+ $visibilityForced = !is_null( self::$hide );
 93+ $hideByUser = $visibilityForced && self::$hide;
 94+ $hideBySetting = in_array( $namespace, $wgHidensNamespaces );
10295
103 - $this->hide = $hideByUser || ( $hideBySetting && $this->hide !== false );
 96+ self::$hide = $hideByUser || ( $hideBySetting && self::$hide !== false );
10497 }
10598
10699 return true;
107100 }
108101
109102 /**
110 - * Callback for the OutputPageBeforeHTML hook
 103+ * Callback for the BeforePageDisplay hook
111104 *
112105 * Removes the namespace from article header and page title
113106 */
114 - public function onBeforePageDisplay( $out ) {
115 - if( $this->hide ) {
 107+ public static function onBeforePageDisplay( $out ) {
 108+ if( self::$hide ) {
116109 $out->setPageTitle( mb_substr( $out->getPageTitle(),
117 - mb_strlen( $this->namespaceText ) + 1 ) );
 110+ mb_strlen( self::$namespaceText ) + 1 ) );
118111 }
119112
120113 return true;

Comments

#Comment by Siebrand (talk | contribs)   08:23, 7 September 2011

Hook methods should be split off the init file into a hooks file/class. I can imagine you're not volunteering for this immediately, but would it be a good idea to add a "@todo FIXME" for that in the file.

Status & tagging log