r40801 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r40800‎ | r40801 | r40802 >
Date:00:49, 14 September 2008
Author:simetrical
Status:old
Tags:
Comment:
Make sure to pass the right types to LinkBegin

If Linker::link() is passed an invalid Title, now that it fails gracefully, we should fail gracefully before passing over to the hook. In theory some hooks might want to override this, but it's unlikely, because any caller that passes a non-Title is probably buggy and should be fixed anyway. This saves unexpected fatal errors and/or having to add "if( !$target instanceof Title ) return true;" to the beginning of every function hooking into this.

Also ensure that $options is an array before passing to the hook, just for convenience.
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -782,7 +782,7 @@
783783 fault values, with a value of false meaning to suppress the attribute.
784784 &$query: the query string to add to the generated URL (the bit after the "?"),
785785 in associative array form, with keys and values unescaped.
786 -&$options: the options. Can include 'known', 'broken', 'noclasses'.
 786+&$options: array of options. Can include 'known', 'broken', 'noclasses'.
787787 &$ret: the value to return if your hook returns false.
788788
789789 'LinkEnd': Used when generating internal and interwiki links in Linker::link(),
Index: trunk/phase3/includes/Linker.php
@@ -170,6 +170,11 @@
171171 */
172172 public function link( $target, $text = null, $customAttribs = array(), $query = array(), $options = array() ) {
173173 wfProfileIn( __METHOD__ );
 174+ if( !$target instanceof Title ) {
 175+ return "<!-- ERROR -->$text";
 176+ }
 177+ $options = (array)$options;
 178+
174179 $ret = null;
175180 if( !wfRunHooks( 'LinkBegin', array( $this, $target, &$text,
176181 &$customAttribs, &$query, &$options, &$ret ) ) ) {
@@ -177,11 +182,6 @@
178183 return $ret;
179184 }
180185
181 - if( !$target instanceof Title ) {
182 - return "<!-- ERROR -->$text";
183 - }
184 - $options = (array)$options;
185 -
186186 # Normalize the Title if it's a special page
187187 $target = $this->normaliseSpecialPage( $target );
188188