Allow undoing edits that change content model if top
authorBrian Wolff <bawolff+wn@gmail.com>
Thu, 8 Sep 2016 22:39:03 +0000 (22:39 +0000)
committerKunal Mehta <legoktm@member.fsf.org>
Fri, 9 Sep 2016 03:19:40 +0000 (20:19 -0700)
This allows people to revert content model changes
using the undo button, provided that we are undoing
the topmost edit (Otherwise it may get confusing
if you try to undo an edit in the middle of the
history that changes content model).

Bug: T145044
Change-Id: Ic528f65d0dc581c4e241a22f19c512e02aeaa9e7

includes/EditPage.php
includes/content/ContentHandler.php

index 4e9aeba..7e4e411 100644 (file)
@@ -562,16 +562,29 @@ class EditPage {
 
                $revision = $this->mArticle->getRevisionFetched();
                // Disallow editing revisions with content models different from the current one
-               if ( $revision && $revision->getContentModel() !== $this->contentModel ) {
-                       $this->displayViewSourcePage(
-                               $this->getContentObject(),
-                               wfMessage(
-                                       'contentmodelediterror',
-                                       $revision->getContentModel(),
-                                       $this->contentModel
-                               )->plain()
-                       );
-                       return;
+               // Undo edits being an exception in order to allow reverting content model changes.
+               if ( $revision
+                       && $revision->getContentModel() !== $this->contentModel
+               ) {
+                       $prevRev = null;
+                       if ( $this->undidRev ) {
+                               $undidRevObj = Revision::newFromId( $this->undidRev );
+                               $prevRev = $undidRevObj ? $undidRevObj->getPrevious() : null;
+                       }
+                       if ( !$this->undidRev
+                               || !$prevRev
+                               || $prevRev->getContentModel() !== $this->contentModel
+                       ) {
+                               $this->displayViewSourcePage(
+                                       $this->getContentObject(),
+                                       wfMessage(
+                                               'contentmodelediterror',
+                                               $revision->getContentModel(),
+                                               $this->contentModel
+                                       )->plain()
+                               );
+                               return;
+                       }
                }
 
                $this->isConflict = false;
@@ -1134,6 +1147,14 @@ class EditPage {
                                                        $oldContent = $this->page->getContent( Revision::RAW );
                                                        $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang );
                                                        $newContent = $content->preSaveTransform( $this->mTitle, $wgUser, $popts );
+                                                       if ( $newContent->getModel() !== $oldContent->getModel() ) {
+                                                               // The undo may change content
+                                                               // model if its reverting the top
+                                                               // edit. This can result in
+                                                               // mismatched content model/format.
+                                                               $this->contentModel = $newContent->getModel();
+                                                               $this->contentFormat = $oldrev->getContentFormat();
+                                                       }
 
                                                        if ( $newContent->equals( $oldContent ) ) {
                                                                # Tell the user that the undo results in no change,
@@ -1264,9 +1285,11 @@ class EditPage {
                        $handler = ContentHandler::getForModelID( $this->contentModel );
 
                        return $handler->makeEmptyContent();
-               } else {
+               } elseif ( !$this->undidRev ) {
                        // Content models should always be the same since we error
-                       // out if they are different before this point.
+                       // out if they are different before this point (in ->edit()).
+                       // The exception being, during an undo, the current revision might
+                       // differ from the prior revision.
                        $logger = LoggerFactory::getInstance( 'editpage' );
                        if ( $this->contentModel !== $rev->getContentModel() ) {
                                $logger->warning( "Overriding content model from current edit {prev} to {new}", [
@@ -1290,9 +1313,8 @@ class EditPage {
                                ] );
                                $this->contentFormat = $rev->getContentFormat();
                        }
-
-                       return $content;
                }
+               return $content;
        }
 
        /**
index bf4cc6d..4e50c8e 100644 (file)
@@ -1017,7 +1017,13 @@ abstract class ContentHandler {
                try {
                        $this->checkModelID( $cur_content->getModel() );
                        $this->checkModelID( $undo_content->getModel() );
-                       $this->checkModelID( $undoafter_content->getModel() );
+                       if ( $current->getId() !== $undo->getId() ) {
+                               // If we are undoing the most recent revision,
+                               // its ok to revert content model changes. However
+                               // if we are undoing a revision in the middle, then
+                               // doing that will be confusing.
+                               $this->checkModelID( $undoafter_content->getModel() );
+                       }
                } catch ( MWException $e ) {
                        // If the revisions have different content models
                        // just return false