r79542 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79541‎ | r79542 | r79543 >
Date:00:42, 4 January 2011
Author:demon
Status:ok (Comments)
Tags:
Comment:
Remove nasty ancient global $title (at least $wgTitle is easier to grep for), its no longer set since r78512. Afaict, this generic-named global is no longer in core.

It's not a title object, it's a string of what title= in the URL. If you want that, use WebRequest::getText() or something. Using this string is *wrong* and any extensions should be fixed (I didn't check because $title is a PITA to grep for). Don't backport to 1.17 because this is trivial, harmless, and may Break Something.

(Also made fileCachedPage private since it has no outside callers and I can't see any reason why you should)
Modified paths:
  • /trunk/phase3/includes/db/Database.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/Database.php
——@@ -2927,8 +2927,8 @@
29282928 return $trygoogle;
29292929 }
29302930
2931 - function fileCachedPage() {
2932 - global $wgTitle, $title, $wgLang, $wgOut;
 2931+ private function fileCachedPage() {
 2932+ global $wgTitle, $wgLang, $wgOut;
29332933
29342934 if ( $wgOut->isDisabled() ) {
29352935 return; // Done already?
——@@ -2937,13 +2937,11 @@
29382938 $mainpage = 'Main Page';
29392939
29402940 if ( $wgLang instanceof Language ) {
2941 - $mainpage = htmlspecialchars( $wgLang->getMessage( 'mainpage' ) );
 2941+ $mainpage = htmlspecialchars( $wgLang->getMessage( 'mainpage' ) );
29422942 }
29432943
29442944 if ( $wgTitle ) {
29452945 $t =& $wgTitle;
2946 - } elseif ( $title ) {
2947 - $t = Title::newFromURL( $title );
29482946 } else {
29492947 $t = Title::newFromText( $mainpage );
29502948 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r85402Kill usage of global $title, non existent in corereedy00:01, 5 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78512Per ^demon, follow-up r78260: introduced MediaWiki::getAction() to get the ac...ialex20:11, 16 December 2010

Comments

#Comment by Ilmari Karonen (talk | contribs)   01:02, 4 January 2011

The files webdav.php and deltav.php in Extension:WebDAV seem to be setting the global $title (like index.php used to before r78512), but that shouldn't matter since they set $wgTitle too (and probably don't interact with the file cache anyway). Also, img_auth.php and maintenance/storage/testCompression.php actually seem to be setting $title to a Title object. Both of these uses seem to be accidental consequences of PHP scoping. I tried grep -RP 'global.*\$title\b' and found no explicit uses of the global $title other than the one you just removed.

#Comment by Platonides (talk | contribs)   11:57, 4 January 2011

I was aware of such instance, but since fileCachedPage() is called in case of an error, maybe it made sense to directly get it from the url...

#Comment by Ilmari Karonen (talk | contribs)   00:05, 6 January 2011

Anyway, just in case it wasn't obvious from my comment, I think this is OK.

Status & tagging log