From 6de4180d0c586e6b83bdfb5d82d107b14f482a7d Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 11 Jun 2012 12:35:46 +0200 Subject: [PATCH] Moved redirect extraction from Title to WikitextContent. All code that wants to know redirects should use the methods from the Content interface, the newFromRedirect methods in Title are deprecated. --- includes/Content.php | 95 +++++++++++++++++++++++++----- includes/ContentHandler.php | 61 +++++++++++++++---- includes/Title.php | 77 ++++-------------------- includes/api/ApiEditPage.php | 8 +-- includes/job/DoubleRedirectJob.php | 7 ++- includes/parser/Parser.php | 5 +- maintenance/checkBadRedirects.php | 2 +- 7 files changed, 152 insertions(+), 103 deletions(-) diff --git a/includes/Content.php b/includes/Content.php index ec81b2ffe7..5965753d0c 100644 --- a/includes/Content.php +++ b/includes/Content.php @@ -582,20 +582,52 @@ abstract class AbstractContent implements Content { * The last element in the array is the final destination after all redirects * have been resolved (up to $wgMaxRedirects times). * + * There is usually no need to override the default behaviour, subclasses that + * want to implement redirects should override getRedirectTarget(). + * * @since WD.1 * * @return Array of Titles, with the destination last + * @note: migrated here from Title::newFromRedirectArray */ public function getRedirectChain() { - return null; + global $wgMaxRedirects; + $title = $this->getRedirectTarget(); + if ( is_null( $title ) ) { + return null; + } + // recursive check to follow double redirects + $recurse = $wgMaxRedirects; + $titles = array( $title ); + while ( --$recurse > 0 ) { + if ( $title->isRedirect() ) { + $page = WikiPage::factory( $title ); + $newtitle = $page->getRedirectTarget(); + } else { + break; + } + // Redirects to some special pages are not permitted + if ( $newtitle instanceOf Title && $newtitle->isValidRedirectTarget() ) { + // the new title passes the checks, so make that our current title so that further recursion can be checked + $title = $newtitle; + $titles[] = $newtitle; + } else { + break; + } + } + return $titles; } /** * Construct the redirect destination from this content and return a Title, * or null if this content doesn't represent a redirect. - * This will only return the immediate redirect target, useful for + * + * This shall only return the immediate redirect target, useful for * the redirect table and other checks that don't need full recursion. * + * This implementation always returns null, subclasses should implement it + * according to their data model. + * * @since WD.1 * * @return Title: The corresponding Title @@ -610,12 +642,17 @@ abstract class AbstractContent implements Content { * This will recurse down $wgMaxRedirects times or until a non-redirect target is hit * in order to provide (hopefully) the Title of the final destination instead of another redirect. * + * There is usually no need to override the default behaviour, subclasses that + * want to implement redirects should override getRedirectTarget(). + * * @since WD.1 * * @return Title + * @note: migrated here from Title::newFromRedirectRecurse */ public function getUltimateRedirectTarget() { - return null; + $titles = $this->getRedirectChain(); + return $titles ? array_pop( $titles ) : null; } /** @@ -943,19 +980,45 @@ class WikitextContent extends TextContent { return new WikitextContent( $plt ); } - public function getRedirectChain() { - $text = $this->getNativeData(); - return Title::newFromRedirectArray( $text ); - } - + /** + * Implement redirect extraction for wikitext. + * + * @return null|Title + * + * @note: migrated here from Title::newFromRedirectInternal() + * + * @see Content::getRedirectTarget + * @see AbstractContent::getRedirectTarget + */ public function getRedirectTarget() { - $text = $this->getNativeData(); - return Title::newFromRedirect( $text ); - } - - public function getUltimateRedirectTarget() { - $text = $this->getNativeData(); - return Title::newFromRedirectRecurse( $text ); + global $wgMaxRedirects; + if ( $wgMaxRedirects < 1 ) { + //redirects are disabled, so quit early + return null; + } + $redir = MagicWord::get( 'redirect' ); + $text = trim( $this->getNativeData() ); + if ( $redir->matchStartAndRemove( $text ) ) { + // Extract the first link and see if it's usable + // Ensure that it really does come directly after #REDIRECT + // Some older redirects included a colon, so don't freak about that! + $m = array(); + if ( preg_match( '!^\s*:?\s*\[{2}(.*?)(?:\|.*?)?\]{2}!', $text, $m ) ) { + // Strip preceding colon used to "escape" categories, etc. + // and URL-decode links + if ( strpos( $m[1], '%' ) !== false ) { + // Match behavior of inline link parsing here; + $m[1] = rawurldecode( ltrim( $m[1], ':' ) ); + } + $title = Title::newFromText( $m[1] ); + // If the title is a redirect to bad special pages or is invalid, return null + if ( !$title instanceof Title || !$title->isValidRedirectTarget() ) { + return null; + } + return $title; + } + } + return null; } /** @@ -971,7 +1034,7 @@ class WikitextContent extends TextContent { * @return bool true if the content is countable */ public function isCountable( $hasLinks = null, Title $title = null ) { - global $wgArticleCountMethod, $wgRequest; + global $wgArticleCountMethod; if ( $this->isRedirect( ) ) { return false; diff --git a/includes/ContentHandler.php b/includes/ContentHandler.php index 2adbee9b2b..d033afc11c 100644 --- a/includes/ContentHandler.php +++ b/includes/ContentHandler.php @@ -80,17 +80,23 @@ abstract class ContentHandler { * @since WD.1 * * @static - * @param string $text the textual representation, will be unserialized to create the Content object - * @param Title $title the title of the page this text belongs to, required as a context for deserialization + * + * @param string $text the textual representation, will be unserialized to create the Content object + * @param null|Title $title the title of the page this text belongs to. Required if $modelId is not provided. * @param null|String $modelId the model to deserialize to. If not provided, $title->getContentModel() is used. - * @param null|String $format the format to use for deserialization. If not given, the model's default format is used. + * @param null|String $format the format to use for deserialization. If not given, the model's default format is used. * * @return Content a Content object representing $text + * * @throw MWException if $model or $format is not supported or if $text can not be unserialized using $format. */ - public static function makeContent( $text, Title $title, $modelId = null, $format = null ) { + public static function makeContent( $text, Title $title = null, $modelId = null, $format = null ) { if ( is_null( $modelId ) ) { + if ( is_null( $title ) ) { + throw new MWException( "Must provide a Title object or a content model ID." ); + } + $modelId = $title->getContentModel(); } @@ -642,6 +648,8 @@ abstract class ContentHandler { $content = $rev->getContent(); $blank = false; + $this->checkModelID( $content->getModel() ); + // If the page is blank, use the text from the previous revision, // which can only be blank if there's a move/import/protect dummy revision involved if ( $content->getSize() == 0 ) { @@ -786,6 +794,10 @@ abstract class ContentHandler { $undo_content = $undo->getContent(); $undoafter_content = $undoafter->getContent(); + $this->checkModelID( $cur_content->getModel() ); + $this->checkModelID( $undo_content->getModel() ); + $this->checkModelID( $undoafter_content->getModel() ); + if ( $cur_content->equals( $undo_content ) ) { // No use doing a merge if it's just a straight revert. return $undoafter_content; @@ -911,6 +923,8 @@ abstract class TextContentHandler extends ContentHandler { * @return ParserOutput representing the HTML form of the text */ public function getParserOutput( Content $content, Title $title, $revId = null, ParserOptions $options = null, $generateHtml = true ) { + $this->checkModelID( $content->getModel() ); + # generic implementation, relying on $this->getHtml() if ( $generateHtml ) $html = $this->getHtml( $content ); @@ -922,13 +936,38 @@ abstract class TextContentHandler extends ContentHandler { /** * Generates an HTML version of the content, for display. - * Used by getParserOutput() to construct a ParserOutput object + * Used by getParserOutput() to construct a ParserOutput object. + * + * This default implementation just calls getHighlightHtml(). Content models that + * have another mapping to HTML (as is the case for markup languages like wikitext) + * should override this method to generate the appropriate html. * * @param Content $content the content to render * - * @return String + * @return String an HTML representation of the content */ - protected abstract function getHtml( Content $content ); + protected function getHtml( Content $content ) { + $this->checkModelID( $content->getModel() ); + + #XXX: hook? + return $this->getHighlighteHtml( $content ); + } + + /** + * Generates a syntax-highlighted version the content, as HTML. + * Used by the default implementation if getHtml(). + * + * @param Content $content the content to render + * + * @return String an HTML representation of the content's markup + */ + protected function getHighlightHtml( Content $content ) { + $this->checkModelID( $content->getModel() ); + + #TODO: use highlighter, if available + #XXX: hook? + return htmlspecialchars( $content->getNativeData() ); + } } @@ -969,6 +1008,8 @@ class WikitextContentHandler extends TextContentHandler { public function getParserOutput( Content $content, Title $title, $revId = null, ParserOptions $options = null, $generateHtml = true ) { global $wgParser; + $this->checkModelID( $content->getModel() ); + if ( !$options ) { $options = new ParserOptions(); } @@ -983,7 +1024,7 @@ class WikitextContentHandler extends TextContentHandler { /** * Returns true because wikitext supports sections. - * + * * @return boolean whether sections are supported. */ public function supportsSections() { @@ -1015,7 +1056,7 @@ class JavaScriptContentHandler extends TextContentHandler { protected function getHtml( Content $content ) { $html = ""; $html .= "
\n";
-		$html .= htmlspecialchars( $content->getNativeData() );
+		$html .= $this->getHighlightHtml( $content );
 		$html .= "\n
\n"; return $html; @@ -1045,7 +1086,7 @@ class CssContentHandler extends TextContentHandler { protected function getHtml( Content $content ) { $html = ""; $html .= "
\n";
-		$html .= htmlspecialchars( $content->getNativeData() );
+		$html .= $this->getHighlightHtml( $content );
 		$html .= "\n
\n"; return $html; diff --git a/includes/Title.php b/includes/Title.php index 28dd886c24..62c604b367 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -370,9 +370,11 @@ class Title { * * @param $text String: Text with possible redirect * @return Title: The corresponding Title + * @deprecated since 1.WD, use Content::getRedirectTarget instead. */ - public static function newFromRedirect( $text ) { #TODO: move to Content class, deprecate here. - return self::newFromRedirectInternal( $text ); + public static function newFromRedirect( $text ) { + $content = ContentHandler::makeContent( $text, null, CONTENT_MODEL_WIKITEXT ); + return $content->getRedirectTarget(); } /** @@ -383,10 +385,11 @@ class Title { * * @param $text String Text with possible redirect * @return Title + * @deprecated since 1.WD, use Content::getUltimateRedirectTarget instead. */ public static function newFromRedirectRecurse( $text ) { - $titles = self::newFromRedirectArray( $text ); - return $titles ? array_pop( $titles ) : null; + $content = ContentHandler::makeContent( $text, null, CONTENT_MODEL_WIKITEXT ); + return $content->getUltimateRedirectTarget(); } /** @@ -397,72 +400,12 @@ class Title { * * @param $text String Text with possible redirect * @return Array of Titles, with the destination last + * @deprecated since 1.WD, use Content::getRedirectChain instead. * @todo: migrate this logic into WikitextContent! */ public static function newFromRedirectArray( $text ) { - global $wgMaxRedirects; - $title = self::newFromRedirectInternal( $text ); - if ( is_null( $title ) ) { - return null; - } - // recursive check to follow double redirects - $recurse = $wgMaxRedirects; - $titles = array( $title ); - while ( --$recurse > 0 ) { - if ( $title->isRedirect() ) { - $page = WikiPage::factory( $title ); - $newtitle = $page->getRedirectTarget(); - } else { - break; - } - // Redirects to some special pages are not permitted - if ( $newtitle instanceOf Title && $newtitle->isValidRedirectTarget() ) { - // the new title passes the checks, so make that our current title so that further recursion can be checked - $title = $newtitle; - $titles[] = $newtitle; - } else { - break; - } - } - return $titles; - } - - /** - * Really extract the redirect destination - * Do not call this function directly, use one of the newFromRedirect* functions above - * - * @param $text String Text with possible redirect - * @return Title - */ - protected static function newFromRedirectInternal( $text ) { - global $wgMaxRedirects; - if ( $wgMaxRedirects < 1 ) { - //redirects are disabled, so quit early - return null; - } - $redir = MagicWord::get( 'redirect' ); - $text = trim( $text ); - if ( $redir->matchStartAndRemove( $text ) ) { - // Extract the first link and see if it's usable - // Ensure that it really does come directly after #REDIRECT - // Some older redirects included a colon, so don't freak about that! - $m = array(); - if ( preg_match( '!^\s*:?\s*\[{2}(.*?)(?:\|.*?)?\]{2}!', $text, $m ) ) { - // Strip preceding colon used to "escape" categories, etc. - // and URL-decode links - if ( strpos( $m[1], '%' ) !== false ) { - // Match behavior of inline link parsing here; - $m[1] = rawurldecode( ltrim( $m[1], ':' ) ); - } - $title = Title::newFromText( $m[1] ); - // If the title is a redirect to bad special pages or is invalid, return null - if ( !$title instanceof Title || !$title->isValidRedirectTarget() ) { - return null; - } - return $title; - } - } - return null; + $content = ContentHandler::makeContent( $text, null, CONTENT_MODEL_WIKITEXT ); + return $content->getRedirectChain(); } /** diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index c1334e9e9b..a9727e2e8d 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -60,7 +60,7 @@ class ApiEditPage extends ApiBase { if ( $titleObj->isRedirect() ) { $oldTitle = $titleObj; - $titles = Title::newFromRedirectArray( Revision::newFromTitle( $oldTitle )->getText( Revision::FOR_THIS_USER ) ); + $titles = Revision::newFromTitle( $oldTitle )->getContent( Revision::FOR_THIS_USER )->getRedirectChain(); // array_shift( $titles ); $redirValues = array(); @@ -110,10 +110,10 @@ class ApiEditPage extends ApiBase { // MediaWiki: pages, though if ( $articleObj->getID() == 0 && $titleObj->getNamespace() != NS_MEDIAWIKI ) { $content = null; - $text = ''; + $text = ''; } else { - $content = $articleObj->getContentObject(); - $text = ContentHandler::getContentText( $content ); #FIXME: serialize?! get format from params?... + $content = $articleObj->getContentObject(); + $text = ContentHandler::getContentText( $content ); #FIXME: serialize?! get format from params?... } if ( !is_null( $params['section'] ) ) { diff --git a/includes/job/DoubleRedirectJob.php b/includes/job/DoubleRedirectJob.php index a9399b507d..8be6db2189 100644 --- a/includes/job/DoubleRedirectJob.php +++ b/includes/job/DoubleRedirectJob.php @@ -94,16 +94,17 @@ class DoubleRedirectJob extends Job { wfDebug( __METHOD__.": target redirect already deleted, ignoring\n" ); return true; } - $text = $targetRev->getText(); - $currentDest = Title::newFromRedirect( $text ); + $content = $targetRev->getContent(); + $currentDest = $content->getRedirectTarget(); if ( !$currentDest || !$currentDest->equals( $this->redirTitle ) ) { wfDebug( __METHOD__.": Redirect has changed since the job was queued\n" ); return true; } # Check for a suppression tag (used e.g. in periodically archived discussions) + $text = ContentHandler::getContentText( $content ); $mw = MagicWord::get( 'staticredirect' ); - if ( $mw->match( $text ) ) { + if ( $mw->match( $text ) ) { #FIXME: add support for this to ContentHandler/Content wfDebug( __METHOD__.": skipping: suppressed with __STATICREDIRECT__\n" ); return true; } diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index 6232bce1af..61d54230c3 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -3567,15 +3567,16 @@ class Parser { break; } $text = $message->plain(); + $content = ContentHandler::makeContent( $text, $title ); #TODO: use Message::content() instead, once that exists } else { break; } - if ( $text === false ) { + if ( !$content ) { break; } # Redirect? $finalTitle = $title; - $title = Title::newFromRedirect( $text ); + $title = $content->getRedirectTarget(); } return array( 'text' => $text, diff --git a/maintenance/checkBadRedirects.php b/maintenance/checkBadRedirects.php index bac2ff69a0..c2e7871581 100644 --- a/maintenance/checkBadRedirects.php +++ b/maintenance/checkBadRedirects.php @@ -46,7 +46,7 @@ class CheckBadRedirects extends Maintenance { $title = Title::makeTitle( $row->page_namespace, $row->page_title ); $rev = Revision::newFromId( $row->page_latest ); if ( $rev ) { - $target = Title::newFromRedirect( $rev->getText() ); + $target = $rev->getContent()->getRedirectTarget(); if ( !$target ) { $this->output( $title->getPrefixedText() . "\n" ); } -- 2.20.1