Merge "Make EditPage robust against null content."
authorReedy <reedy@wikimedia.org>
Thu, 18 Oct 2012 19:14:30 +0000 (19:14 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 18 Oct 2012 19:14:30 +0000 (19:14 +0000)
includes/DefaultSettings.php
includes/EditPage.php
includes/content/TextContent.php
includes/content/WikitextContent.php
includes/filebackend/FSFileBackend.php
includes/media/MediaTransformOutput.php
tests/phpunit/includes/CssContentTest.php
tests/phpunit/includes/JavascriptContentTest.php
tests/phpunit/includes/TextContentTest.php
tests/phpunit/includes/WikitextContentTest.php
tests/phpunit/includes/filerepo/FileBackendTest.php

index 05dc1d3..fdddfc9 100644 (file)
@@ -6324,6 +6324,22 @@ $wgContentHandlerTextFallback = 'ignore';
  */
 $wgContentHandlerUseDB = false;
 
+/**
+ * Determines which types of text are parsed as wikitext. This does not imply that these kinds
+ * of texts are also rendered as wikitext, it only means that links, magic words, etc will have
+ * the effect on the database they would have on a wikitext page.
+ *
+ * @todo: On the long run, it would be nice to put categories etc into a separate structure,
+ * or at least parse only the contents of comments in the scripts.
+ *
+ * @since 1.21
+ */
+$wgTextModelsToParse = array(
+       CONTENT_MODEL_WIKITEXT,    // Just for completeness, wikitext will always be parsed.
+       CONTENT_MODEL_JAVASCRIPT,  // Make categories etc work, people put them into comments.
+       CONTENT_MODEL_CSS,         // Make categories etc work, people put them into comments.
+);
+
 /**
  * Whether the user must enter their password to change their e-mail address
  *
index 697daf0..89b3701 100644 (file)
@@ -471,7 +471,7 @@ class EditPage {
                $content = $this->getContentObject();
 
                # Use the normal message if there's nothing to display
-               if ( $this->firsttime && $content->isEmpty() ) {
+               if ( $this->firsttime && ( $content === false || $content->isEmpty() ) ) {
                        $action = $this->mTitle->exists() ? 'edit' :
                                ( $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage' );
                        throw new PermissionsError( $action, $permErrors );
@@ -1901,20 +1901,30 @@ class EditPage {
        }
 
        /**
-        * Gets an editable textual representation of the given Content object.
+        * Gets an editable textual representation of $content.
         * The textual representation can be turned by into a Content object by the
         * toEditContent() method.
         *
+        * If $content is null or false or a string, $content is returned unchanged.
+        *
         * If the given Content object is not of a type that can be edited using the text base EditPage,
         * an exception will be raised. Set $this->allowNonTextContent to true to allow editing of non-textual
         * content.
         *
-        * @param Content $content
+        * @param Content|null|false|string $content
         * @return String the editable text form of the content.
         *
         * @throws MWException if $content is not an instance of TextContent and $this->allowNonTextContent is not true.
         */
-       protected function toEditText( Content $content ) {
+       protected function toEditText( $content ) {
+               if ( $content === null || $content === false ) {
+                       return $content;
+               }
+
+               if ( is_string( $content ) ) {
+                       return $content;
+               }
+
                if ( !$this->allowNonTextContent && !( $content instanceof TextContent ) ) {
                        throw new MWException( "This content model can not be edited as text: "
                                                                . ContentHandler::getLocalizedName( $content->getModel() ) );
@@ -2608,16 +2618,26 @@ HTML
                                                        $this->section, $textboxContent,
                                                        $this->summary, $this->edittime );
 
-               ContentHandler::runLegacyHooks( 'EditPageGetDiffText', array( $this, &$newContent ) );
-               wfRunHooks( 'EditPageGetDiffContent', array( $this, &$newContent ) );
+               if ( $newContent ) {
+                       ContentHandler::runLegacyHooks( 'EditPageGetDiffText', array( $this, &$newContent ) );
+                       wfRunHooks( 'EditPageGetDiffContent', array( $this, &$newContent ) );
 
-               $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang );
-               $newContent = $newContent->preSaveTransform( $this->mTitle, $wgUser, $popts );
+                       $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang );
+                       $newContent = $newContent->preSaveTransform( $this->mTitle, $wgUser, $popts );
+               }
 
                if ( ( $oldContent && !$oldContent->isEmpty() ) || ( $newContent && !$newContent->isEmpty() ) ) {
                        $oldtitle = wfMessage( $oldtitlemsg )->parse();
                        $newtitle = wfMessage( 'yourtext' )->parse();
 
+                       if ( !$oldContent ) {
+                               $oldContent = $newContent->getContentHandler()->makeEmptyContent();
+                       }
+
+                       if ( !$newContent ) {
+                               $newContent = $oldContent->getContentHandler()->makeEmptyContent();
+                       }
+
                        $de = $oldContent->getContentHandler()->createDifferenceEngine( $this->mArticle->getContext() );
                        $de->setContent( $oldContent, $newContent );
 
index b561b90..8e832ec 100644 (file)
@@ -116,6 +116,22 @@ class TextContent extends AbstractContent {
                return $this->getNativeData();
        }
 
+       /**
+        * Returns a Content object with pre-save transformations applied.
+        * This implementation just trims trailing whitespace.
+        *
+        * @param $title Title
+        * @param $user User
+        * @param $popts ParserOptions
+        * @return Content
+        */
+       public function preSaveTransform( Title $title, User $user, ParserOptions $popts ) {
+               $text = $this->getNativeData();
+               $pst = rtrim( $text );
+
+               return ( $text === $pst ) ? $this : new WikitextContent( $pst );
+       }
+
        /**
         * Diff this content object with another content object..
         *
@@ -164,7 +180,19 @@ class TextContent extends AbstractContent {
                $revId = null,
                ParserOptions $options = null, $generateHtml = true
        ) {
-               # Generic implementation, relying on $this->getHtml()
+               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
+                       $po = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+               } else {
+                       $po = new ParserOutput();
+               }
 
                if ( $generateHtml ) {
                        $html = $this->getHtml();
@@ -172,7 +200,7 @@ class TextContent extends AbstractContent {
                        $html = '';
                }
 
-               $po = new ParserOutput( $html );
+               $po->setText( $html );
                return $po;
        }
 
index 89a9fe9..8f1381f 100644 (file)
@@ -116,8 +116,9 @@ class WikitextContent extends TextContent {
 
                $text = $this->getNativeData();
                $pst = $wgParser->preSaveTransform( $text, $title, $user, $popts );
+               rtrim( $pst );
 
-               return new WikitextContent( $pst );
+               return ( $text === $pst ) ? $this : new WikitextContent( $pst );
        }
 
        /**
index 0922919..dd43f82 100644 (file)
@@ -670,8 +670,8 @@ class FSFileBackend extends FileBackendStore {
 
                foreach ( $params['srcs'] as $src ) {
                        $source = $this->resolveToFSPath( $src );
-                       if ( $source === null ) {
-                               $fsFiles[$src] = null; // invalid path
+                       if ( $source === null || !is_file( $source ) ) {
+                               $fsFiles[$src] = null; // invalid path or file does not exist
                        } else {
                                $fsFiles[$src] = new FSFile( $source );
                        }
@@ -700,7 +700,9 @@ class FSFileBackend extends FileBackendStore {
                                } else {
                                        $tmpPath = $tmpFile->getPath();
                                        // Copy the source file over the temp file
+                                       wfSuppressWarnings();
                                        $ok = copy( $source, $tmpPath );
+                                       wfRestoreWarnings();
                                        if ( !$ok ) {
                                                $tmpFiles[$src] = null;
                                        } else {
index 69bdc2f..97a2d1d 100644 (file)
@@ -196,7 +196,10 @@ abstract class MediaTransformOutput {
         * @return array
         */
        public function getDescLinkAttribs( $title = null, $params = '' ) {
-               $query = $this->page ? ( 'page=' . urlencode( $this->page ) ) : '';
+               $query = '';
+               if ( $this->page && $this->page !== 1 ) {
+                         $query = 'page=' . urlencode( $this->page );
+               }
                if( $params ) {
                        $query .= $query ? '&'.$params : $params;
                }
index 4f79dfc..b6e8d29 100644 (file)
@@ -15,8 +15,20 @@ class CssContentTest extends JavascriptContentTest {
 
        public function dataGetParserOutput() {
                return array(
-                       array("MediaWiki:Test.css", null, "hello <world>\n",
-                               "<pre class=\"mw-code mw-css\" dir=\"ltr\">\nhello &lt;world&gt;\n\n</pre>"),
+                       array(
+                               "MediaWiki:Test.css",
+                               null,
+                               "hello <world>\n",
+                               "<pre class=\"mw-code mw-css\" dir=\"ltr\">\nhello &lt;world&gt;\n\n</pre>" ),
+
+                       array(
+                               "MediaWiki:Test.css",
+                               null,
+                               "/* hello [[world]] */\n",
+                               "<pre class=\"mw-code mw-css\" dir=\"ltr\">\n/* hello [[world]] */\n\n</pre>",
+                               array( 'Links' => array( // NOTE: assumes default settings for $wgTextModelsToParse
+                                       array( 'World' => 0 ) ) ) ),
+
                        // @todo: more...?
                );
        }
index b45caa2..d3810af 100644 (file)
@@ -15,8 +15,20 @@ class JavascriptContentTest extends TextContentTest {
 
        public function dataGetParserOutput() {
                return array(
-                       array("MediaWiki:Test.js", null, "hello <world>\n",
-                                       "<pre class=\"mw-code mw-js\" dir=\"ltr\">\nhello &lt;world&gt;\n\n</pre>"),
+                       array(
+                               "MediaWiki:Test.js",
+                               null,
+                               "hello <world>\n",
+                               "<pre class=\"mw-code mw-js\" dir=\"ltr\">\nhello &lt;world&gt;\n\n</pre>" ),
+
+                       array(
+                               "MediaWiki:Test.js",
+                               null,
+                               "hello(); // [[world]]\n",
+                               "<pre class=\"mw-code mw-js\" dir=\"ltr\">\nhello(); // [[world]]\n\n</pre>",
+                               array( 'Links' => array( // NOTE: assumes default settings for $wgTextModelsToParse
+                                                       array( 'World' => 0 ) ) ) ),
+
                        // @todo: more...?
                );
        }
@@ -89,6 +101,9 @@ class JavascriptContentTest extends TextContentTest {
                        array( 'hello \'\'this\'\' is <nowiki>~~~</nowiki>',
                                'hello \'\'this\'\' is <nowiki>~~~</nowiki>',
                        ),
+                       array( " Foo \n ",
+                               " Foo",
+                       ),
                );
        }
 
index ee17a75..10934b4 100644 (file)
@@ -27,7 +27,11 @@ class TextContentTest extends MediaWikiTestCase {
 
        public function dataGetParserOutput() {
                return array(
-                       array("TextContentTest_testGetParserOutput", CONTENT_MODEL_TEXT, "hello ''world'' & stuff\n", "hello ''world'' &amp; stuff"),
+                       array(
+                               "TextContentTest_testGetParserOutput",
+                               CONTENT_MODEL_TEXT,
+                               "hello ''world'' & [[stuff]]\n", "hello ''world'' &amp; [[stuff]]",
+                               array( 'Links' => array() ) ),
                        // @todo: more...?
                );
        }
@@ -35,7 +39,7 @@ class TextContentTest extends MediaWikiTestCase {
        /**
         * @dataProvider dataGetParserOutput
         */
-       public function testGetParserOutput( $title, $model, $text, $expectedHtml ) {
+       public function testGetParserOutput( $title, $model, $text, $expectedHtml, $expectedFields = null ) {
                $title = Title::newFromText( $title );
                $content = ContentHandler::makeContent( $text, $title, $model );
 
@@ -45,13 +49,32 @@ class TextContentTest extends MediaWikiTestCase {
                $html = preg_replace( '#<!--.*?-->#sm', '', $html ); // strip comments
 
                $this->assertEquals( $expectedHtml, trim( $html ) );
+
+               if ( $expectedFields ) {
+                       foreach ( $expectedFields as $field => $exp ) {
+                               $f = 'get' . ucfirst( $field );
+                               $v = call_user_func( array( $po, $f ) );
+
+                               if ( is_array( $exp ) ) {
+                                       $this->assertArrayEquals( $exp, $v );
+                               } else {
+                                       $this->assertEquals( $exp, $v );
+                               }
+                       }
+               }
+
                // @todo: assert more properties
        }
 
        public function dataPreSaveTransform() {
                return array(
-                       array( 'hello this is ~~~',
-                              "hello this is ~~~",
+                       array( #0: no signature resolution
+                               "hello this is ~~~",
+                               "hello this is ~~~",
+                       ),
+                       array( #1: rtrim
+                               " Foo \n ",
+                               " Foo",
                        ),
                );
        }
index b2d3bdf..c1332a6 100644 (file)
@@ -21,12 +21,12 @@ class WikitextContentTest extends TextContentTest {
 
        public function dataGetSecondaryDataUpdates() {
                return array(
-                       array("WikitextContentTest_testGetSecondaryDataUpdates_1",
+                       array( "WikitextContentTest_testGetSecondaryDataUpdates_1",
                                CONTENT_MODEL_WIKITEXT, "hello ''world''\n",
                                array( 'LinksUpdate' => array(  'mRecursive' => true,
                                                                'mLinks' => array() ) )
                        ),
-                       array("WikitextContentTest_testGetSecondaryDataUpdates_2",
+                       array( "WikitextContentTest_testGetSecondaryDataUpdates_2",
                                CONTENT_MODEL_WIKITEXT, "hello [[world test 21344]]\n",
                                array( 'LinksUpdate' => array(  'mRecursive' => true,
                                                                'mLinks' => array( array( 'World_test_21344' => 0 ) ) ) )
@@ -174,6 +174,10 @@ just a test"
                        array( 'hello \'\'this\'\' is <nowiki>~~~</nowiki>',
                               'hello \'\'this\'\' is <nowiki>~~~</nowiki>',
                        ),
+                       array( // rtrim
+                               " Foo \n ",
+                               " Foo",
+                       ),
                );
        }
 
index f159d5d..7201eec 100644 (file)
@@ -1113,6 +1113,32 @@ class FileBackendTest extends MediaWikiTestCase {
                return $cases;
        }
 
+       public function testGetLocalCopyAndReference404() {
+               $this->backend = $this->singleBackend;
+               $this->tearDownFiles();
+               $this->doTestGetLocalCopyAndReference404();
+               $this->tearDownFiles();
+
+               $this->backend = $this->multiBackend;
+               $this->tearDownFiles();
+               $this->doTestGetLocalCopyAndReference404();
+               $this->tearDownFiles();
+       }
+
+       public function doTestGetLocalCopyAndReference404() {
+               $backendName = $this->backendClass();
+
+               $base = self::baseStorePath();
+
+               $tmpFile = $this->backend->getLocalCopy( array(
+                       'src' => "$base/unittest-cont1/not-there" ) );
+               $this->assertEquals( null, $tmpFile, "Local copy of not existing file is null ($backendName)." );
+
+               $tmpFile = $this->backend->getLocalReference( array(
+                       'src' => "$base/unittest-cont1/not-there" ) );
+               $this->assertEquals( null, $tmpFile, "Local ref of not existing file is null ($backendName)." );
+       }
+
        /**
         * @dataProvider provider_testPrepareAndClean
         */