From 04a7e6291ee4559b3031a91cb7c53eeaa7310421 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 13 Jan 2006 00:29:20 +0000 Subject: [PATCH] * (bug 1103) Fix up redirect handling for images, categories Redirects are now followed from the top-level, outside of the Article content loading and viewing, for clarity and consistency. --- RELEASE-NOTES | 3 + includes/Article.php | 210 +++++++++++++++----------------- includes/EditPage.php | 16 +-- includes/ImagePage.php | 21 ++-- includes/SpecialBooksources.php | 6 +- includes/Wiki.php | 86 ++++++++----- maintenance/dumpHTML.inc | 1 + maintenance/importLogs.php | 2 +- 8 files changed, 178 insertions(+), 167 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index b1e9bd85c9..fe6037e1a5 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -474,6 +474,9 @@ fully support the editing toolbar, but was found to be too confusing. * Override $wgLocaltimezone in parser tests for us outside Iceland and UK * Fix extra whitespace at end of Wiki.php, DESTROYS XML OUTPUT * Remove redundant 'echo' statements from MonoBook.php +* (bug 1103) Fix up redirect handling for images, categories + Redirects are now followed from the top-level, outside of the Article + content loading and viewing, for clarity and consistency. === Caveats === diff --git a/includes/Article.php b/includes/Article.php index ac18a182f0..9f1d3b8fab 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -49,6 +49,59 @@ class Article { $this->mOldId = $oldId; $this->clear(); } + + /** + * Tell the page view functions that this view was redirected + * from another page on the wiki. + * @param Title $from + */ + function setRedirectedFrom( $from ) { + $this->mRedirectedFrom = $from; + } + + /** + * @return mixed false, Title of in-wiki target, or string with URL + */ + function followRedirect() { + $text = $this->getContent(); + $rt = Title::newFromRedirect( $text ); + + # process if title object is valid and not special:userlogout + if( $rt ) { + if( $rt->getInterwiki() != '' ) { + if( $rt->isLocal() ) { + // Offsite wikis need an HTTP redirect. + // + // This can be hard to reverse and may produce loops, + // so they may be disabled in the site configuration. + + $source = $this->mTitle->getFullURL( 'redirect=no' ); + return $rt->getFullURL( 'rdfrom=' . urlencode( $source ) ); + } + } else { + if( $rt->getNamespace() == NS_SPECIAL ) { + // Gotta hand redirects to special pages differently: + // Fill the HTTP response "Location" header and ignore + // the rest of the page we're on. + // + // This can be hard to reverse, so they may be disabled. + + if( $rt->getNamespace() == NS_SPECIAL && $rt->getText() == 'Userlogout' ) { + // rolleyes + } else { + return $rt->getFullURL(); + } + } elseif( $rt->exists() ) { + // Internal redirects can be handled relatively gracefully. + // We may have to change to another Article subclass, though. + return $rt; + } + } + } + + // No or invalid redirect + return false; + } /** * get the title object of the article @@ -66,7 +119,8 @@ class Article { $this->mContentLoaded = false; $this->mCurID = $this->mUser = $this->mCounter = -1; # Not loaded - $this->mRedirectedFrom = $this->mUserText = + $this->mRedirectedFrom = null; # Title object if set + $this->mUserText = $this->mTimestamp = $this->mComment = $this->mFileCache = ''; $this->mGoodAdjustment = $this->mTotalAdjustment = 0; $this->mTouched = '19700101000000'; @@ -77,13 +131,17 @@ class Article { } /** - * Note that getContent/loadContent may follow redirects if - * not told otherwise, and so may cause a change to mTitle. + * Note that getContent/loadContent do not follow redirects anymore. + * If you need to fetch redirectable content easily, try + * the shortcut in Article::followContent() + * + * @fixme There are still side-effects in this! + * In general, you should use the Revision class, not Article, + * to fetch text for purposes other than page views. * - * @param $noredir * @return Return the text of this revision */ - function getContent( $noredir ) { + function getContent() { global $wgRequest, $wgUser, $wgOut; # Get variables from query string :P @@ -117,7 +175,7 @@ class Article { return "
$ret
"; } else { - $this->loadContent( $noredir ); + $this->loadContent(); # check if we're displaying a [[User talk:x.x.x.x]] anonymous talk page if ( $this->mTitle->getNamespace() == NS_USER_TALK && $wgUser->isIP($this->mTitle->getText()) && @@ -295,14 +353,13 @@ class Article { /** * Load the revision (including text) into this object */ - function loadContent( $noredir = false ) { + function loadContent() { global $wgOut, $wgRequest; if ( $this->mContentLoaded ) return; # Query variables :P $oldid = $this->getOldID(); - $redirect = $wgRequest->getVal( 'redirect' ); $fname = 'Article::loadContent'; @@ -311,10 +368,8 @@ class Article { $t = $this->mTitle->getPrefixedText(); - $noredir = $noredir || ($wgRequest->getVal( 'redirect' ) == 'no') - || $wgRequest->getCheck( 'rdfrom' ); $this->mOldId = $oldid; - $this->fetchContent( $oldid, $noredir, true ); + $this->fetchContent( $oldid ); } @@ -385,12 +440,11 @@ class Article { /** * Get text of an article from database + * Does *NOT* follow redirects. * @param int $oldid 0 for whatever the latest revision is - * @param bool $noredir Set to false to follow redirects - * @param bool $globalTitle Set to true to change the global $wgTitle object when following redirects or other unexpected title changes * @return string */ - function fetchContent( $oldid = 0, $noredir = true, $globalTitle = false ) { + function fetchContent( $oldid = 0 ) { if ( $this->mContentLoaded ) { return $this->mContent; } @@ -404,10 +458,6 @@ class Article { if( $oldid ) { $t .= ',oldid='.$oldid; } - if( isset( $redirect ) ) { - $redirect = ($redirect == 'no') ? 'no' : 'yes'; - $t .= ',redirect='.$redirect; - } $this->mContent = wfMsg( 'missingarticle', $t ) ; if( $oldid ) { @@ -439,53 +489,6 @@ class Article { } } - # If we got a redirect, follow it (unless we've been told - # not to by either the function parameter or the query - if ( !$oldid && !$noredir ) { - $rt = Title::newFromRedirect( $revision->getText() ); - # process if title object is valid and not special:userlogout - if ( $rt && ! ( $rt->getNamespace() == NS_SPECIAL && $rt->getText() == 'Userlogout' ) ) { - # Gotta hand redirects to special pages differently: - # Fill the HTTP response "Location" header and ignore - # the rest of the page we're on. - global $wgDisableHardRedirects; - if( $globalTitle && !$wgDisableHardRedirects ) { - global $wgOut; - if ( $rt->getInterwiki() != '' && $rt->isLocal() ) { - $source = $this->mTitle->getFullURL( 'redirect=no' ); - $wgOut->redirect( $rt->getFullURL( 'rdfrom=' . urlencode( $source ) ) ) ; - return false; - } - if ( $rt->getNamespace() == NS_SPECIAL ) { - $wgOut->redirect( $rt->getFullURL() ); - return false; - } - } - if( $rt->getInterwiki() == '' ) { - $redirData = $this->pageDataFromTitle( $dbr, $rt ); - if( $redirData ) { - $redirRev = Revision::newFromId( $redirData->page_latest ); - if( !is_null( $redirRev ) ) { - $this->mRedirectedFrom = $this->mTitle->getPrefixedText(); - $this->mTitle = $rt; - $data = $redirData; - $this->loadPageData( $data ); - $revision = $redirRev; - } - } - } - } - } - - # if the title's different from expected, update... - if( $globalTitle ) { - global $wgTitle; - if( !$this->mTitle->equals( $wgTitle ) ) { - $wgTitle = $this->mTitle; - } - } - - # Back to the business at hand... $this->mContent = $revision->getText(); $this->mUser = $revision->getUser(); @@ -502,19 +505,6 @@ class Article { return $this->mContent; } - /** - * Gets the article text without using so many damn globals - * - * Used by maintenance/importLogs.php - * - * @param int $oldid - * @param bool $noredir Whether to follow redirects - * @return mixed the content (string) or false on error - */ - function getContentWithoutUsingSoManyDamnGlobals( $oldid = 0, $noredir = false ) { - return $this->fetchContent( $oldid, $noredir, false ); - } - /** * Read/write accessor to select FOR UPDATE * @@ -791,6 +781,30 @@ class Article { wfIncrStats( 'pcache_miss_stub' ); } + $wasRedirected = false; + if ( isset( $this->mRedirectedFrom ) ) { + // This is an internally redirected page view. + // We'll need a backlink to the source page for navigation. + if ( wfRunHooks( 'ArticleViewRedirect', array( &$this ) ) ) { + $sk = $wgUser->getSkin(); + $redir = $sk->makeKnownLinkObj( $this->mRedirectedFrom, '', 'redirect=no' ); + $s = wfMsg( 'redirectedfrom', $redir ); + $wgOut->setSubtitle( $s ); + $wasRedirected = true; + } + } elseif ( !empty( $rdfrom ) ) { + // This is an externally redirected view, from some other wiki. + // If it was reported from a trusted site, supply a backlink. + global $wgRedirectSources; + if( $wgRedirectSources && preg_match( $wgRedirectSources, $rdfrom ) ) { + $sk = $wgUser->getSkin(); + $redir = $sk->makeExternalLink( $rdfrom, $rdfrom ); + $s = wfMsg( 'redirectedfrom', $redir ); + $wgOut->setSubtitle( $s ); + $wasRedirected = true; + } + } + $outputDone = false; if ( $pcache ) { if ( $wgOut->tryParserCache( $this, $wgUser ) ) { @@ -798,21 +812,17 @@ class Article { } } if ( !$outputDone ) { - $text = $this->getContent( false ); # May change mTitle by following a redirect + $text = $this->getContent(); if ( $text === false ) { # Failed to load, replace text with error message $t = $this->mTitle->getPrefixedText(); if( $oldid ) { $t .= ',oldid='.$oldid; } - if( isset( $redirect ) ) { - $redirect = ($redirect == 'no') ? 'no' : 'yes'; - $t .= ',redirect='.$redirect; - } $text = wfMsg( 'missingarticle', $t ); } - # Another whitelist check in case oldid or redirects are altering the title + # Another whitelist check in case oldid is altering the title if ( !$this->mTitle->userCanRead() ) { $wgOut->loginToUse(); $wgOut->output(); @@ -825,32 +835,6 @@ class Article { $this->setOldSubtitle( isset($this->mOldId) ? $this->mOldId : $oldid ); $wgOut->setRobotpolicy( 'noindex,follow' ); } - $wasRedirected = false; - if ( '' != $this->mRedirectedFrom ) { - if ( wfRunHooks( 'ArticleViewRedirect', array( &$this ) ) ) { - $sk = $wgUser->getSkin(); - $redir = $sk->makeKnownLink( $this->mRedirectedFrom, '', 'redirect=no' ); - $s = wfMsg( 'redirectedfrom', $redir ); - $wgOut->setSubtitle( $s ); - - // Check the parser cache again, for the target page - if( $pcache ) { - if( $wgOut->tryParserCache( $this, $wgUser ) ) { - $outputDone = true; - } - } - $wasRedirected = true; - } - } elseif ( !empty( $rdfrom ) ) { - global $wgRedirectSources; - if( $wgRedirectSources && preg_match( $wgRedirectSources, $rdfrom ) ) { - $sk = $wgUser->getSkin(); - $redir = $sk->makeExternalLink( $rdfrom, $rdfrom ); - $s = wfMsg( 'redirectedfrom', $redir ); - $wgOut->setSubtitle( $s ); - $wasRedirected = true; - } - } } if( !$outputDone ) { /** @@ -1366,7 +1350,7 @@ class Article { $userAbort = ignore_user_abort( true ); } - $oldtext = $this->getContent( true ); + $oldtext = $this->getContent(); $oldsize = strlen( $oldtext ); $newsize = strlen( $text ); $lastRevision = 0; @@ -1954,7 +1938,7 @@ class Article { return false; } - $u = new SiteStatsUpdate( 0, 1, -(int)$this->isCountable( $this->getContent( true ) ), -1 ); + $u = new SiteStatsUpdate( 0, 1, -(int)$this->isCountable( $this->getContent() ), -1 ); array_push( $wgDeferredUpdateList, $u ); $linksTo = $this->mTitle->getLinksTo(); @@ -2050,7 +2034,7 @@ class Article { } if ( wfReadOnly() ) { - $wgOut->readOnlyPage( $this->getContent( true ) ); + $wgOut->readOnlyPage( $this->getContent() ); return; } if( !$wgUser->matchEditToken( $wgRequest->getVal( 'token' ), diff --git a/includes/EditPage.php b/includes/EditPage.php index fbb89a8963..3a4d7619a3 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -53,7 +53,7 @@ class EditPage { if ( !$wgUseMetadataEdit ) return ; if ( $wgMetadataWhitelist == '' ) return ; $s = '' ; - $t = $this->mArticle->getContent ( true ) ; + $t = $this->mArticle->getContent(); # MISSING : filtering @@ -90,7 +90,7 @@ class EditPage { $sat = array () ; # stand-alone-templates; must be lowercase $wl_title = Title::newFromText ( $wgMetadataWhitelist ) ; $wl_article = new Article ( $wl_title ) ; - $wl = explode ( "\n" , $wl_article->getContent(true) ) ; + $wl = explode ( "\n" , $wl_article->getContent() ) ; foreach ( $wl AS $x ) { $isentry = false ; @@ -176,7 +176,7 @@ class EditPage { if ( ! $this->mTitle->userCanEdit() ) { wfDebug( "$fname: user can't edit\n" ); - $wgOut->readOnlyPage( $this->mArticle->getContent( true ), true ); + $wgOut->readOnlyPage( $this->mArticle->getContent(), true ); wfProfileOut( $fname ); return; } @@ -197,7 +197,7 @@ class EditPage { return; } else { wfDebug( "$fname: read-only page\n" ); - $wgOut->readOnlyPage( $this->mArticle->getContent( true ), true ); + $wgOut->readOnlyPage( $this->mArticle->getContent(), true ); wfProfileOut( $fname ); return; } @@ -215,7 +215,7 @@ class EditPage { } else if ( $this->diff ) { $this->formtype = 'diff'; } else { - $wgOut->readOnlyPage( $this->mArticle->getContent( true ) ); + $wgOut->readOnlyPage( $this->mArticle->getContent() ); wfProfileOut( $fname ); return; } @@ -651,7 +651,7 @@ class EditPage { */ function initialiseForm() { $this->edittime = $this->mArticle->getTimestamp(); - $this->textbox1 = $this->mArticle->getContent( true ); + $this->textbox1 = $this->mArticle->getContent(); $this->summary = ''; if ( !$this->mArticle->exists() && $this->mArticle->mTitle->getNamespace() == NS_MEDIAWIKI ) $this->textbox1 = wfMsgWeirdKey ( $this->mArticle->mTitle->getText() ) ; @@ -685,7 +685,7 @@ class EditPage { $wgOut->addWikiText( wfMsg( 'explainconflict' ) ); $this->textbox2 = $this->textbox1; - $this->textbox1 = $this->mArticle->getContent( true ); + $this->textbox1 = $this->mArticle->getContent(); $this->edittime = $this->mArticle->getTimestamp(); } else { @@ -1108,7 +1108,7 @@ END } else { # if user want to see preview when he edit an article if( $wgUser->getOption('previewonfirst') and ($this->textbox1 == '')) { - $this->textbox1 = $this->mArticle->getContent(true); + $this->textbox1 = $this->mArticle->getContent(); } $toparse = $this->textbox1; diff --git a/includes/ImagePage.php b/includes/ImagePage.php index 8d58285ee1..cb0c86777b 100644 --- a/includes/ImagePage.php +++ b/includes/ImagePage.php @@ -20,10 +20,14 @@ class ImagePage extends Article { /* private */ var $img; // Image object this page is shown for var $mExtraDescription = false; + /** + * Handler for action=render + * Include body text only; none of the image extras + */ function render() { global $wgOut; - $wgOut->setArticleBodyOnly(true); - $wgOut->addWikitext($this->getContent(true)); + $wgOut->setArticleBodyOnly( true ); + $wgOut->addWikitext( $this->getContent() ); } function view() { @@ -147,16 +151,15 @@ class ImagePage extends Article { /** * Overloading Article's getContent method. - * Omit noarticletext if sharedupload - * - * @param $noredir If true, do not follow redirects + * + * Omit noarticletext if sharedupload; text will be fetched from the + * shared upload server if possible. */ - function getContent( $noredir ) - { - if ( $this->img && $this->img->fromSharedDirectory && 0 == $this->getID() ) { + function getContent() { + if( $this->img && $this->img->fromSharedDirectory && 0 == $this->getID() ) { return ''; } - return Article::getContent( $noredir ); + return Article::getContent(); } function openShowImage() { diff --git a/includes/SpecialBooksources.php b/includes/SpecialBooksources.php index 47e70dccea..ae8df7332b 100644 --- a/includes/SpecialBooksources.php +++ b/includes/SpecialBooksources.php @@ -53,9 +53,9 @@ class BookSourceList { # First, see if we have a custom list setup in # [[Wikipedia:Book sources]] or equivalent. $bstitle = Title::makeTitleSafe( NS_PROJECT, wfMsg( "booksources" ) ); - $bsarticle = new Article( $bstitle ); - if( $bsarticle->exists() ) { - $bstext = $bsarticle->getContent( false ); + $revision = Revision::newFromTitle( $bstitle ); + if( $revision ) { + $bstext = $revision->getText(); if( $bstext ) { $bstext = str_replace( "MAGICNUMBER", $this->mIsbn, $bstext ); $wgOut->addWikiText( $bstext ); diff --git a/includes/Wiki.php b/includes/Wiki.php index 52f0a8189a..996a5cf832 100644 --- a/includes/Wiki.php +++ b/includes/Wiki.php @@ -46,7 +46,13 @@ class MediaWiki { $article = NULL; if ( !$this->initializeSpecialCases( $title, $output, $request ) ) { $article = $this->initializeArticle( $title, $request ); - $this->performAction( $output, $article, $title, $user, $request ); + if( is_object( $article ) ) { + $this->performAction( $output, $article, $title, $user, $request ); + } elseif( is_string( $article ) ) { + $output->redirect( $article ); + } else { + wfDebugDieBacktrace( "Shouldn't happen: MediaWiki::initializeArticle() returned neither an object nor a URL" ); + } } wfProfileOut( 'MediaWiki::initialize' ); return $article; @@ -158,46 +164,60 @@ class MediaWiki { } /** - * Initialize the object to be known as $wgArticle for "standard" actions + * Create an Article object of the appropriate class for the given page. + * @param Title $title + * @return Article */ - function initializeArticle( &$title, $request ) { - - wfProfileIn( 'MediaWiki::initializeArticle' ); - - $action = $this->getVal('Action'); - + function articleFromTitle( $title ) { if( NS_MEDIA == $title->getNamespace() ) { + // FIXME: where should this go? $title = Title::makeTitle( NS_IMAGE, $title->getDBkey() ); } - $ns = $title->getNamespace(); + switch( $title->getNamespace() ) { + case NS_IMAGE: + require_once( 'includes/ImagePage.php' ); + return new ImagePage( $title ); + case NS_CATEGORY: + require_once( 'includes/CategoryPage.php' ); + return new CategoryPage( $title ); + default: + return new Article( $title ); + } + } - /* Namespace might change when using redirects */ - $article = new Article( $title ); - if( $action == 'view' && !$request->getVal( 'oldid' ) ) { - $rTitle = Title::newFromRedirect( $article->fetchContent() ); - if( $rTitle ) { - /* Reload from the page pointed to later */ - $article->mContentLoaded = false; - $ns = $rTitle->getNamespace(); - $wasRedirected = true; + /** + * Initialize the object to be known as $wgArticle for "standard" actions + * Create an Article object for the page, following redirects if needed. + * @param Title $title + * @param Request $request + * @param string $action + * @return mixed an Article, or a string to redirect to another URL + */ + function initializeArticle( $title, $request ) { + wfProfileIn( 'MediaWiki::initializeArticle' ); + + $action = $this->getVal('Action'); + $article = $this->articleFromTitle( $title ); + + // Namespace might change when using redirects + if( $action == 'view' && !$request->getVal( 'oldid' ) && $request->getVal( 'redirect' ) != 'no' ) { + $target = $article->followRedirect(); + if( is_string( $target ) ) { + global $wgDisableHardRedirects; + if( !$wgDisableHardRedirects ) { + // we'll need to redirect + return $target; } - } - - /* Categories and images are handled by a different class */ - if( $ns == NS_IMAGE ) { - $b4 = $title->getPrefixedText(); - unset( $article ); - require_once( 'includes/ImagePage.php' ); - $article = new ImagePage( $title ); - if( isset( $wasRedirected ) && $request->getVal( 'redirect' ) != 'no' ) { - $article->mTitle = $rTitle; - $article->mRedirectedFrom = $b4; } - } elseif( $ns == NS_CATEGORY ) { - unset( $article ); - require_once( 'includes/CategoryPage.php' ); - $article = new CategoryPage( $title ); + if( is_object( $target ) ) { + // evil globals hack! + global $wgTitle; + $wgTitle = $target; + + $article = $this->articleFromTitle( $target ); + $article->setRedirectedFrom( $title ); + } } wfProfileOut( 'MediaWiki::initializeArticle' ); return $article; diff --git a/maintenance/dumpHTML.inc b/maintenance/dumpHTML.inc index d81d7c186a..ee42525051 100644 --- a/maintenance/dumpHTML.inc +++ b/maintenance/dumpHTML.inc @@ -370,6 +370,7 @@ class DumpHTML { $wgOut->setParserOptions( new ParserOptions ); SpecialPage::executePath( $wgTitle ); } else { + /** @todo merge with Wiki.php code */ if ( $ns == NS_IMAGE ) { $wgArticle = new ImagePage( $wgTitle ); } elseif ( $ns == NS_CATEGORY ) { diff --git a/maintenance/importLogs.php b/maintenance/importLogs.php index 221e5f9c07..6187c2e6e1 100644 --- a/maintenance/importLogs.php +++ b/maintenance/importLogs.php @@ -17,7 +17,7 @@ foreach( LogPage::validTypes() as $type ) { $page = LogPage::logName( $type ); $log = new Article( Title::makeTitleSafe( NS_PROJECT, $page ) ); - $text = $log->getContentWithoutUsingSoManyDamnGlobals(); + $text = $log->fetchContent(); $importer = new LogImporter( $type ); $importer->dummy = true; -- 2.20.1