Revert "Introducing ContentGetParserOutput hook."
authorOri.livneh <ori@wikimedia.org>
Mon, 10 Jun 2013 19:13:00 +0000 (19:13 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 10 Jun 2013 19:13:00 +0000 (19:13 +0000)
This change removed getHtml(), but not the call to it from fillParserOutput.
It has a few other issues that I'll flag in-line after deploying the revert.

This reverts commit fda090a7e7f85f146ec84798fda45232ab67613e

Bug: 49398
Change-Id: Idcef8d4cedc7c03f72bc1743e0f89cc5ed4ad7a7

docs/hooks.txt
includes/content/AbstractContent.php
includes/content/CssContent.php
includes/content/JavaScriptContent.php
includes/content/TextContent.php
includes/content/WikitextContent.php
tests/phpunit/includes/content/ContentHandlerTest.php
tests/phpunit/includes/content/CssContentTest.php
tests/phpunit/includes/content/TextContentTest.php

index 02c9fad..66b5068 100644 (file)
@@ -801,19 +801,6 @@ content model name, but no entry for that model exists in $wgContentHandlers.
 $modeName: the requested content model name
 &$handler: set this to a ContentHandler object, if desired.
 
-'ContentGetParserOutput': Customize parser output for a given content object,
-called by AbstractContent::getParserOutput. May be used to override the normal
-model-specific rendering of page content.
-$content: The Content to render
-$title: Title of the page, as context
-$revId: The revision ID, as context
-$options: ParserOptions for rendering. To avoid confusing the parser cache,
-the output can only depend on parameters provided to this hook function, not on global state.
-$generateHtml: boolean, indicating whether full HTML should be generated. If false,
-generation of HTML may be skipped, but other information should still be present in the
-ParserOutput object.
-&$output: ParserOutput, to manipulate or replace
-
 'ConvertContent': Called by AbstractContent::convert when a conversion to another
 content model is requested.
 $content: The Content object to be converted.
@@ -2007,7 +1994,7 @@ $title : Current Title object being displayed in search results.
 $article: The article object corresponding to the page
 
 'ShowRawCssJs': Customise the output of raw CSS and JavaScript in page views.
-DEPRECATED, use the ContentGetParserOutput hook instead!
+DEPRECATED, use the ContentHandler facility to handle CSS and JavaScript!
 $text: Text being shown
 $title: Title of the custom script/stylesheet page
 $output: Current OutputPage object
index 8c6e24a..137efb8 100644 (file)
@@ -441,57 +441,4 @@ abstract class AbstractContent implements Content {
                wfRunHooks( 'ConvertContent', array( $this, $toModel, $lossy, &$result ) );
                return $result;
        }
-
-       /**
-        * Returns a ParserOutput object, filled by a call to fillParserOutput().
-        * Subclasses that want to control the parser output may override this, or
-        * they can override fillParserOutput(). If they override getParserOutput(),
-        * itself, they should take care to call the ContentGetParserOutput hook.
-        *
-        * @param $title Title Context title for parsing
-        * @param $revId int|null Revision ID (for {{REVISIONID}})
-        * @param $options ParserOptions|null Parser options
-        * @param $generateHtml bool Whether or not to generate HTML
-        *
-        * @return ParserOutput representing the HTML form of the text
-        */
-       public function getParserOutput( Title $title, $revId = null,
-               ParserOptions $options = null, $generateHtml = true
-       ) {
-               # Generic implementation, relying on $this->getHtml()
-
-               if ( $options === null ) {
-                       $options = $this->getContentHandler()->makeParserOptions( 'canonical' );
-               }
-
-               $po = new ParserOutput();
-
-               if ( wfRunHooks( 'ContentGetParserOutput',
-                       array( $this, $title, $revId, $options, $generateHtml, &$po ) ) ) {
-
-                       $this->fillParserOutput( $title, $revId, $options, $generateHtml, $po );
-               }
-
-               return $po;
-       }
-
-       /**
-        * Fills the provided ParserOutput object with the HTML returned by getHtml().
-        * Subclasses should override this (or getParserOutput) appropriately.
-        * This placeholder implementation always throws an exception.
-        *
-        * @param $title        Title Context title for parsing
-        * @param $revId        int|null Revision ID (for {{REVISIONID}})
-        * @param $options      ParserOptions|null Parser options
-        * @param $generateHtml bool Whether or not to generate HTML
-        * @param &$output      ParserOutput The output object to fill (reference).
-        *
-        * @throws MWException
-        */
-       protected function fillParserOutput( Title $title, $revId,
-               ParserOptions $options, $generateHtml, ParserOutput &$output
-       ) {
-               // Don't make abstract, so subclasses that override getParserOutput() directly don't fail.
-               throw new MWException( 'Subclasses of AbstractContent must override fillParserOutput!' );
-       }
 }
index a25bd76..03cc2d0 100644 (file)
@@ -57,7 +57,7 @@ class CssContent extends TextContent {
        protected function getHtml() {
                $html = "";
                $html .= "<pre class=\"mw-code mw-css\" dir=\"ltr\">\n";
-               $html .= htmlspecialchars( $this->getNativeData() );
+               $html .= $this->getHighlightHtml();
                $html .= "\n</pre>\n";
 
                return $html;
index 247de6d..2ae572b 100644 (file)
@@ -58,7 +58,7 @@ class JavaScriptContent extends TextContent {
        protected function getHtml() {
                $html = "";
                $html .= "<pre class=\"mw-code mw-js\" dir=\"ltr\">\n";
-               $html .= htmlspecialchars( $this->getNativeData() );
+               $html .= $this->getHighlightHtml();
                $html .= "\n</pre>\n";
 
                return $html;
index a6ee7b0..f66dacd 100644 (file)
@@ -189,27 +189,32 @@ class TextContent extends AbstractContent {
        }
 
        /**
-        * Fills the provided ParserOutput object with the HTML returned by getHtml().
-        *
-        * Content models in $wgTextModelsToParse will be parsed as wikitext to process links,
-        * magic words, etc.
-        *
-        * Subclasses may override this to provide custom rendering.
+        * Returns a generic ParserOutput object, wrapping the HTML returned by
+        * getHtml().
         *
         * @param $title Title Context title for parsing
         * @param int|null $revId Revision ID (for {{REVISIONID}})
         * @param $options ParserOptions|null Parser options
         * @param bool $generateHtml Whether or not to generate HTML
-        * @param $output ParserOutput The output object to fill (reference).
+        *
+        * @return ParserOutput representing the HTML form of the text
         */
-       protected function fillParserOutput( Title $title, $revId,
-               ParserOptions $options, $generateHtml, ParserOutput &$output
+       public function getParserOutput( Title $title,
+               $revId = null,
+               ParserOptions $options = null, $generateHtml = true
        ) {
                global $wgParser, $wgTextModelsToParse;
 
+               if ( !$options ) {
+                       //NOTE: use canonical options per default to produce cacheable output
+                       $options = $this->getContentHandler()->makeParserOptions( 'canonical' );
+               }
+
                if ( in_array( $this->getModel(), $wgTextModelsToParse ) ) {
-                       // parse just to get links etc into the database, HTML is replaced below.
-                       $output = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+                       // parse just to get links etc into the database
+                       $po = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+               } else {
+                       $po = new ParserOutput();
                }
 
                if ( $generateHtml ) {
@@ -218,25 +223,34 @@ class TextContent extends AbstractContent {
                        $html = '';
                }
 
-               $output->setText( $html );
+               $po->setText( $html );
+               return $po;
        }
 
        /**
         * Generates an HTML version of the content, for display. Used by
         * getParserOutput() to construct a ParserOutput object.
         *
-        * This default implementation runs the text returned by $this->getNativeData()
-        * through htmlspecialchars and tried to convert line breaks and indentation to HTML..
+        * 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.
         *
         * @return string An HTML representation of the content
         */
-       public static function convertWhiteSpaceToHTML( $msg ) {
-               $msg = htmlspecialchars( $msg );
-               $msg = preg_replace( '/^ /m', '&#160;', $msg );
-               $msg = preg_replace( '/ $/m', '&#160;', $msg );
-               $msg = preg_replace( '/  /', '&#160; ', $msg );
-               $msg = str_replace( "\n", '<br />', $msg );
-               return $msg;
+       protected function getHtml() {
+               return $this->getHighlightHtml();
+       }
+
+       /**
+        * Generates a syntax-highlighted version of the content, as HTML.
+        * Used by the default implementation of getHtml().
+        *
+        * @return string an HTML representation of the content's markup
+        */
+       protected function getHighlightHtml() {
+               # TODO: make Highlighter interface, use highlighter here, if available
+               return htmlspecialchars( $this->getNativeData() );
        }
 
        /**
index d821d9d..26337db 100644 (file)
@@ -277,21 +277,28 @@ class WikitextContent extends TextContent {
         * using $wgParser.
         *
         * @since    1.21
-        * @see AbstractContent::fillParserOutput().
         *
         * @param $title Title
         * @param int $revId Revision to pass to the parser (default: null)
         * @param $options ParserOptions (default: null)
         * @param bool $generateHtml (default: false)
-        * @param &$output ParserOutput representing the HTML form of the text,
-        *           may be manipulated or replaced.
+        *
+        * @internal param \IContextSource|null $context
+        * @return ParserOutput representing the HTML form of the text
         */
-       protected function fillParserOutput( Title $title, $revId,
-                       ParserOptions $options, $generateHtml, ParserOutput &$output
+       public function getParserOutput( Title $title,
+               $revId = null,
+               ParserOptions $options = null, $generateHtml = true
        ) {
                global $wgParser;
 
-               $output = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+               if ( !$options ) {
+                       //NOTE: use canonical options per default to produce cacheable output
+                       $options = $this->getContentHandler()->makeParserOptions( 'canonical' );
+               }
+
+               $po = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+               return $po;
        }
 
        protected function getHtml() {
index 2904472..c345513 100644 (file)
@@ -431,18 +431,4 @@ class DummyContentForTesting extends AbstractContent {
        public function getParserOutput( Title $title, $revId = null, ParserOptions $options = null, $generateHtml = true ) {
                return new ParserOutput( $this->getNativeData() );
        }
-
-       /**
-        * @see AbstractContent::fillParserOutput()
-        *
-        * @param $title        Title Context title for parsing
-        * @param $revId        int|null Revision ID (for {{REVISIONID}})
-        * @param $options      ParserOptions|null Parser options
-        * @param $generateHtml bool Whether or not to generate HTML
-        * @param $output       ParserOutput The output object to fill (reference).
-        */
-       protected function fillParserOutput( Title $title, $revId,
-                       ParserOptions $options, $generateHtml, ParserOutput &$output ) {
-               $output = new ParserOutput( $this->getNativeData() );
-       }
 }
index 61716f9..1c45820 100644 (file)
@@ -5,7 +5,7 @@
  * @group Database
  *        ^--- needed, because we do need the database to test link updates
  */
-class CssContentTest extends JavaScriptContentTest {
+class CssContentTest extends MediaWikiTestCase {
 
        protected function setUp() {
                parent::setUp();
index d7dde37..c7138b7 100644 (file)
@@ -7,21 +7,14 @@
  */
 class TextContentTest extends MediaWikiLangTestCase {
        protected $context;
-       protected $savedContentGetParserOutput;
 
        protected function setUp() {
-               global $wgHooks;
-
                parent::setUp();
 
                // Anon user
                $user = new User();
                $user->setName( '127.0.0.1' );
 
-               $this->context = new RequestContext( new FauxRequest() );
-               $this->context->setTitle( Title::newFromText( 'Test' ) );
-               $this->context->setUser( $user );
-
                $this->setMwGlobals( array(
                        'wgUser' => $user,
                        'wgTextModelsToParse' => array(
@@ -33,22 +26,9 @@ class TextContentTest extends MediaWikiLangTestCase {
                        'wgAlwaysUseTidy' => false,
                ) );
 
-               // bypass hooks that force custom rendering
-               if ( isset( $wgHooks['ContentGetParserOutput'] )  ) {
-                       $this->savedContentGetParserOutput = $wgHooks['ContentGetParserOutput'];
-                       unset( $wgHooks['ContentGetParserOutput'] );
-               }
-       }
-
-       public function teardown() {
-               global $wgHooks;
-
-               // restore hooks that force custom rendering
-               if ( $this->savedContentGetParserOutput !== null ) {
-                       $wgHooks['ContentGetParserOutput'] = $this->savedContentGetParserOutput;
-               }
-
-               parent::teardown();
+               $this->context = new RequestContext( new FauxRequest() );
+               $this->context->setTitle( Title::newFromText( 'Test' ) );
+               $this->context->setUser( $user );
        }
 
        public function newContent( $text ) {