From 74fe09ed4dcc59ae660fe9ba794111bf03638b97 Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 20 Aug 2012 19:28:20 +0200 Subject: [PATCH] Bug 39509: Function for running legacy hooks. ContentHandler::runLegacyHooks can be used to run hooks that don't supprot Content objects yet. runLegacyHooks will issue a warning and take case of serialization/unserialization of the content as appropriate. Changeset 2: rebased. Change-Id: I31109061110f87c38bdeebf30d520c8e1241bb29 --- includes/Article.php | 21 +++++-- includes/ContentHandler.php | 56 +++++++++++++++++++ includes/EditPage.php | 33 +++-------- includes/WikiPage.php | 37 +++++------- includes/diff/DifferenceEngine.php | 8 +-- tests/phpunit/includes/ContentHandlerTest.php | 21 ++++++- 6 files changed, 115 insertions(+), 61 deletions(-) diff --git a/includes/Article.php b/includes/Article.php index dc6294f2b7..f791edffd0 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -391,7 +391,7 @@ class Article extends Page { $content = $this->fetchContentObject(); $this->mContent = ContentHandler::getContentText( $content ); #@todo: get rid of mContent everywhere! - wfRunHooks( 'ArticleAfterFetchContent', array( &$this, &$this->mContent ) ); #BC cruft, deprecated! + ContentHandler::runLegacyHooks( 'ArticleAfterFetchContent', array( &$this, &$this->mContent ) ); wfProfileOut( __METHOD__ ); @@ -679,10 +679,16 @@ class Article extends Page { wfDebug( __METHOD__ . ": showing CSS/JS source\n" ); $this->showCssOrJsPage(); $outputDone = true; - } elseif( !wfRunHooks( 'ArticleContentViewCustom', array( $this->fetchContentObject(), $this->getTitle(), $outputPage ) ) ) { + } elseif( !wfRunHooks( 'ArticleContentViewCustom', + array( $this->fetchContentObject(), $this->getTitle(), + $outputPage ) ) ) { + # Allow extensions do their own custom view for certain pages $outputDone = true; - } elseif( Hooks::isRegistered( 'ArticleViewCustom' ) && !wfRunHooks( 'ArticleViewCustom', array( $this->fetchContent(), $this->getTitle(), $outputPage ) ) ) { #FIXME: fetchContent() is deprecated! + } elseif( !ContentHandler::runLegacyHooks( 'ArticleViewCustom', + array( $this->fetchContentObject(), $this->getTitle(), + $outputPage ) ) ) { + # Allow extensions do their own custom view for certain pages $outputDone = true; } else { @@ -693,7 +699,8 @@ class Article extends Page { # Viewing a redirect page (e.g. with parameter redirect=no) $outputPage->addHTML( $this->viewRedirect( $rt ) ); # Parse just to get categories, displaytitle, etc. - $this->mParserOutput = $content->getParserOutput( $this->getTitle(), $oldid, $parserOptions, false ); + $this->mParserOutput = $content->getParserOutput( $this->getTitle(), $oldid, + $parserOptions, false ); $outputPage->addParserOutputNoText( $this->mParserOutput ); $outputDone = true; } @@ -830,7 +837,7 @@ class Article extends Page { } // Give hooks a chance to customise the output - if ( !Hooks::isRegistered('ShowRawCssJs') || wfRunHooks( 'ShowRawCssJs', array( $this->fetchContent(), $this->getTitle(), $wgOut ) ) ) { #FIXME: fetchContent() is deprecated + if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->fetchContentObject(), $this->getTitle(), $wgOut ) ) ) { $po = $this->mContentObject->getParserOutput( $this->getTitle() ); $wgOut->addHTML( $po->getText() ); } @@ -1704,8 +1711,10 @@ class Article extends Page { * @return ParserOutput or false if the given revsion ID is not found */ public function getParserOutput( $oldid = null, User $user = null ) { + //XXX: bypasses mParserOptions and thus setParserOptions() + $user = is_null( $user ) ? $this->getContext()->getUser() : $user; - $parserOptions = $this->mPage->makeParserOptions( $user ); //XXX: bypasses mParserOptions and thus setParserOptions() + $parserOptions = $this->mPage->makeParserOptions( $user ); return $this->mPage->getParserOutput( $parserOptions, $oldid ); } diff --git a/includes/ContentHandler.php b/includes/ContentHandler.php index e460eeef21..a0a555218e 100644 --- a/includes/ContentHandler.php +++ b/includes/ContentHandler.php @@ -825,6 +825,62 @@ abstract class ContentHandler { public function supportsSections() { return false; } + + /** + * Call a legacy hook that uses text instead of Content objects. + * Will log a warning when a matching hook function is registered. + * If the textual representation of the content is changed by the + * hook function, a new Content object is constructed from the new + * text. + * + * @param $event String: event name + * @param $args Array: parameters passed to hook functions + * + * @return Boolean True if no handler aborted the hook + */ + public static function runLegacyHooks( $event, $args = array() ) { + if ( !Hooks::isRegistered( $event ) ) { + return true; // nothing to do here + } + + wfWarn( "Using obsolete hook $event" ); + + // convert Content objects to text + $contentObjects = array(); + $contentTexts = array(); + + foreach ( $args as $k => $v ) { + if ( $v instanceof Content ) { + /* @var Content $v */ + + $contentObjects[$k] = $v; + + $v = $v->serialize(); + $contentTexts[ $k ] = $v; + $args[ $k ] = $v; + } + } + + // call the hook functions + $ok = wfRunHooks( $event, $args ); + + // see if the hook changed the text + foreach ( $contentTexts as $k => $orig ) { + /* @var Content $content */ + + $modified = $args[ $k ]; + $content = $contentObjects[$k]; + + if ( $modified !== $orig ) { + // text was changed, create updated Content object + $content = $content->getContentHandler()->unserializeContent( $modified ); + } + + $args[ $k ] = $content; + } + + return $ok; + } } /** diff --git a/includes/EditPage.php b/includes/EditPage.php index 3e38fb8ca9..09e5f15350 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1457,8 +1457,10 @@ class EditPage { } // Run post-section-merge edit filter - if ( !wfRunHooks( 'EditFilterMerged', array( $this, $content->serialize( $this->content_format ), &$this->hookError, $this->summary ) ) - || !wfRunHooks( 'EditFilterMergedContent', array( $this, $content, &$this->hookError, $this->summary ) ) ) { + $hook_args = array( $this, $content, &$this->hookError, $this->summary ); + + if ( !ContentHandler::runLegacyHooks( 'EditFilterMerged', $hook_args ) + || !wfRunHooks( 'EditFilterMergedContent', $hook_args ) ) { # Error messages etc. could be handled within the hook... $status->fatal( 'hookaborted' ); $status->value = self::AS_HOOK_ERROR; @@ -2547,18 +2549,7 @@ HTML $this->section, $textboxContent, $this->summary, $this->edittime ); - # hanlde legacy text-based hook - $newtext_orig = $newContent->serialize( $this->content_format ); - $newtext = $newtext_orig; #clone - wfRunHooks( 'EditPageGetDiffText', array( $this, &$newtext ) ); - - if ( $newtext != $newtext_orig ) { - #if the hook changed the text, create a new Content object accordingly. - #XXX: handle parse errors ? - $newContent = ContentHandler::makeContent( $newtext, $this->getTitle(), - $newContent->getModel() ); - } - + ContentHandler::runLegacyHooks( 'EditPageGetDiffText', array( $this, &$newContent ) ); wfRunHooks( 'EditPageGetDiffContent', array( $this, &$newContent ) ); $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang ); @@ -2862,17 +2853,9 @@ HTML $content = $content->addSectionHeader( $this->summary ); } - $toparse_orig = $content->serialize( $this->content_format ); - $toparse = $toparse_orig; - wfRunHooks( 'EditPageGetPreviewText', array( $this, &$toparse ) ); - - if ( $toparse !== $toparse_orig ) { - #hook changed the text, create new Content object - $content = ContentHandler::makeContent( $toparse, $this->getTitle(), - $this->content_model, $this->content_format ); - } - - wfRunHooks( 'EditPageGetPreviewContent', array( $this, &$content ) ); + $hook_args = array( $this, &$content ); + ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args ); + wfRunHooks( 'EditPageGetPreviewContent', $hook_args ); $parserOptions->enableLimitReport(); diff --git a/includes/WikiPage.php b/includes/WikiPage.php index a4afda7ef4..0318f2d8d9 100644 --- a/includes/WikiPage.php +++ b/includes/WikiPage.php @@ -1623,24 +1623,13 @@ class WikiPage extends Page implements IDBAccessObject { $flags = $this->checkFlags( $flags ); - # call legacy hook - $hook_ok = wfRunHooks( 'ArticleContentSave', array( &$this, &$user, &$content, &$summary, - $flags & EDIT_MINOR, null, null, &$flags, &$status ) ); + # handle hook + $hook_args = array( &$this, &$user, &$content, &$summary, + $flags & EDIT_MINOR, null, null, &$flags, &$status ); - if ( $hook_ok && Hooks::isRegistered( 'ArticleSave' ) ) { # avoid serialization overhead if the hook isn't present - $content_text = $content->serialize(); - $txt = $content_text; # clone + if ( !wfRunHooks( 'ArticleContentSave', $hook_args ) + || !ContentHandler::runLegacyHooks( 'ArticleSave', $hook_args ) ) { - $hook_ok = wfRunHooks( 'ArticleSave', array( &$this, &$user, &$txt, &$summary, - $flags & EDIT_MINOR, null, null, &$flags, &$status ) ); #TODO: survey extensions using this hook - - if ( $txt !== $content_text ) { - # if the text changed, unserialize the new version to create an updated Content object. - $content = $content->getContentHandler()->unserializeContent( $txt ); - } - } - - if ( !$hook_ok ) { wfDebug( __METHOD__ . ": ArticleSave or ArticleSaveContent hook aborted save!\n" ); if ( $status->isOK() ) { @@ -1869,11 +1858,11 @@ class WikiPage extends Page implements IDBAccessObject { # Update links, etc. $this->doEditUpdates( $revision, $user, array( 'created' => true ) ); - wfRunHooks( 'ArticleInsertComplete', array( &$this, &$user, $serialized, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision ) ); + $hook_args = array( &$this, &$user, $content, $summary, + $flags & EDIT_MINOR, null, null, &$flags, $revision ); - wfRunHooks( 'ArticleContentInsertComplete', array( &$this, &$user, $content, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision ) ); + ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $hook_args ); + wfRunHooks( 'ArticleContentInsertComplete', $hook_args ); } # Do updates right now unless deferral was requested @@ -1884,11 +1873,11 @@ class WikiPage extends Page implements IDBAccessObject { // Return the new revision (or null) to the caller $status->value['revision'] = $revision; - wfRunHooks( 'ArticleSaveComplete', array( &$this, &$user, $serialized, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ) ); + $hook_args = array( &$this, &$user, $content, $summary, + $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ); - wfRunHooks( 'ArticleContentSaveComplete', array( &$this, &$user, $content, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ) ); + ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $hook_args ); + wfRunHooks( 'ArticleContentSaveComplete', $hook_args ); # Promote user to any groups they meet the criteria for $user->addAutopromoteOnceGroups( 'onEdit' ); diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index f76d9f0cab..3a624a8b5b 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -510,13 +510,12 @@ class DifferenceEngine extends ContextSource { $out->setRevisionTimestamp( $this->mNewRev->getTimestamp() ); $out->setArticleFlag( true ); - #NOTE: only needed for B/C: custom rendering of JS/CSS via hook + // NOTE: only needed for B/C: custom rendering of JS/CSS via hook if ( $this->mNewPage->isCssJsSubpage() || $this->mNewPage->isCssOrJsPage() ) { // Stolen from Article::view --AG 2007-10-11 // Give hooks a chance to customise the output // @TODO: standardize this crap into one function - if ( !Hook::isRegistered( 'ShowRawCssJs' ) - || wfRunHooks( 'ShowRawCssJs', array( ContentHandler::getContentText( $this->mNewContent ), $this->mNewPage, $out ) ) ) { + if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->mNewContent, $this->mNewPage, $out ) ) ) { // NOTE: deprecated hook, B/C only // use the content object's own rendering $po = $this->mContentObject->getParserOutput(); @@ -524,8 +523,7 @@ class DifferenceEngine extends ContextSource { } } elseif( !wfRunHooks( 'ArticleContentViewCustom', array( $this->mNewContent, $this->mNewPage, $out ) ) ) { // Handled by extension - } elseif( Hooks::isRegistered( 'ArticleViewCustom' ) - && !wfRunHooks( 'ArticleViewCustom', array( ContentHandler::getContentText( $this->mNewContent ), $this->mNewPage, $out ) ) ) { + } elseif( !ContentHandler::runLegacyHooks( 'ArticleViewCustom', array( $this->mNewContent, $this->mNewPage, $out ) ) ) { // NOTE: deprecated hook, B/C only // Handled by extension } else { diff --git a/tests/phpunit/includes/ContentHandlerTest.php b/tests/phpunit/includes/ContentHandlerTest.php index afeddf88df..01bba65726 100644 --- a/tests/phpunit/includes/ContentHandlerTest.php +++ b/tests/phpunit/includes/ContentHandlerTest.php @@ -236,6 +236,26 @@ class ContentHandlerTest extends MediaWikiTestCase { public function testSupportsSections() { $this->markTestIncomplete( "not yet implemented" ); } + + public function testRunLegacyHooks() { + Hooks::register( 'testRunLegacyHooks', __CLASS__ . '::dummyHookHandler' ); + + $content = new WikitextContent( 'test text' ); + $ok = ContentHandler::runLegacyHooks( 'testRunLegacyHooks', array( 'foo', &$content, 'bar' ) ); + + $this->assertTrue( $ok, "runLegacyHooks should have returned true" ); + $this->assertEquals( "TEST TEXT", $content->getNativeData() ); + } + + public static function dummyHookHandler( $foo, &$text, $bar ) { + if ( $text === null || $text === false ) { + return false; + } + + $text = strtoupper( $text ); + + return true; + } } class DummyContentHandlerForTesting extends ContentHandler { @@ -389,4 +409,3 @@ class DummyContentForTesting extends AbstractContent { return new ParserOutput( $this->getNativeData() ); } } - -- 2.20.1