(bug 42915) make MovePage aware of whether redirects are supported.
authordaniel <daniel.kinzler@wikimedia.de>
Thu, 13 Dec 2012 16:26:43 +0000 (17:26 +0100)
committerdaniel <daniel.kinzler@wikimedia.de>
Thu, 20 Dec 2012 18:00:35 +0000 (19:00 +0100)
Special:Movepage and ApiMovePage should be aware whether the present content
model supports redirects, and only offer to keep a redirect on moving
of it's supported.

Change-Id: I2d02b2668ea8ebe39387b79d165f6cf1037303b9

includes/api/ApiMove.php
includes/content/ContentHandler.php
includes/content/WikitextContentHandler.php
includes/specials/SpecialMovepage.php

index 9d73562..6c53e99 100644 (file)
@@ -82,9 +82,16 @@ class ApiMove extends ApiBase {
                }
 
                $r = array( 'from' => $fromTitle->getPrefixedText(), 'to' => $toTitle->getPrefixedText(), 'reason' => $params['reason'] );
-               if ( !$params['noredirect'] || !$user->isAllowed( 'suppressredirect' ) ) {
+
+               if ( $fromTitle->exists() ) {
+                       //NOTE: we assume that if the old title exists, it's because it was re-created as
+                       // a redirect to the new title. This is not safe, but what we did before was
+                       // even worse: we just determined whether a redirect should have been created,
+                       // and reported that it was created if it should have, without any checks.
+                       // Also note that isRedirect() is unreliable because of bug 37209.
                        $r['redirectcreated'] = '';
                }
+
                if( $toTitleExists ) {
                        $r['moveoverredirect'] = '';
                }
index 282a7ba..a2db328 100644 (file)
@@ -442,6 +442,9 @@ abstract class ContentHandler {
         * This default implementation always returns null. Subclasses supporting redirects
         * must override this method.
         *
+        * Note that subclasses that override this method to return a Content object
+        * should also override supportsRedirects() to return true.
+        *
         * @since 1.21
         *
         * @param Title $destination the page to redirect to.
@@ -965,15 +968,30 @@ abstract class ContentHandler {
 
        /**
         * Returns true if this content model supports sections.
-        *
         * This default implementation returns false.
         *
+        * Content models that return true here should also implement
+        * Content::getSection, Content::replaceSection, etc. to handle sections..
+        *
         * @return boolean whether sections are supported.
         */
        public function supportsSections() {
                return false;
        }
 
+       /**
+        * Returns true if this content model supports redirects.
+        * This default implementation returns false.
+        *
+        * Content models that return true here should also implement
+        * ContentHandler::makeRedirectContent to return a Content object.
+        *
+        * @return boolean whether redirects are supported.
+        */
+       public function supportsRedirects() {
+               return false;
+       }
+
        /**
         * Logs a deprecation warning, visible if $wgDevelopmentWarnings, but only if
         * self::$enableDeprecationWarnings is set to true.
index 4ec20ef..6ff01e2 100644 (file)
@@ -59,6 +59,17 @@ class WikitextContentHandler extends TextContentHandler {
                return new WikitextContent( $redirectText );
        }
 
+       /**
+        * Returns true because wikitext supports redirects.
+        *
+        * @see ContentHandler::supportsRedirects
+        *
+        * @return boolean whether redirects are supported.
+        */
+       public function supportsRedirects() {
+               return true;
+       }
+
        /**
         * Returns true because wikitext supports sections.
         *
index ce2633f..66fd918 100644 (file)
@@ -254,6 +254,8 @@ class MovePageForm extends UnlistedSpecialPage {
                        }
                }
 
+               $handler = ContentHandler::getForTitle( $this->oldTitle );
+
                $out->addHTML(
                         Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->getLocalURL( 'action=submit' ), 'id' => 'movepage' ) ) .
                         Xml::openElement( 'fieldset' ) .
@@ -309,7 +311,7 @@ class MovePageForm extends UnlistedSpecialPage {
                        );
                }
 
-               if ( $user->isAllowed( 'suppressredirect' ) ) {
+               if ( $user->isAllowed( 'suppressredirect' ) && $handler->supportsRedirects() ) {
                        $out->addHTML( "
                                <tr>
                                        <td></td>
@@ -447,7 +449,11 @@ class MovePageForm extends UnlistedSpecialPage {
                        }
                }
 
-               if ( $user->isAllowed( 'suppressredirect' ) ) {
+               $handler = ContentHandler::getForTitle( $ot );
+
+               if ( !$handler->supportsRedirects() ) {
+                       $createRedirect = false;
+               } elseif ( $user->isAllowed( 'suppressredirect' ) ) {
                        $createRedirect = $this->leaveRedirect;
                } else {
                        $createRedirect = true;
@@ -477,7 +483,18 @@ class MovePageForm extends UnlistedSpecialPage {
                $oldText = $ot->getPrefixedText();
                $newText = $nt->getPrefixedText();
 
-               $msgName = $createRedirect ? 'movepage-moved-redirect' : 'movepage-moved-noredirect';
+               if ( $ot->exists() ) {
+                       //NOTE: we assume that if the old title exists, it's because it was re-created as
+                       // a redirect to the new title. This is not safe, but what we did before was
+                       // even worse: we just determined whether a redirect should have been created,
+                       // and reported that it was created if it should have, without any checks.
+                       // Also note that isRedirect() is unreliable because of bug 37209.
+                       $msgName = 'movepage-moved-redirect';
+               } else {
+                       $msgName = 'movepage-moved-noredirect';
+               }
+
+
                $out->addHTML( $this->msg( 'movepage-moved' )->rawParams( $oldLink,
                        $newLink )->params( $oldText, $newText )->parseAsBlock() );
                $out->addWikiMsg( $msgName );