From: Brian Wolff <bawolff+wn@gmail.com>
Date: Thu, 8 Sep 2016 22:39:03 +0000 (+0000)
Subject: Allow undoing edits that change content model if top
X-Git-Tag: 1.31.0-rc.0~5691^2
X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=386bed1ffd9da7eb5830e0c2386e5dc79e261dfe;p=lhc%2Fweb%2Fwiklou.git

Allow undoing edits that change content model if top

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
---

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 4e9aebaf63..7e4e411d5e 100644
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -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;
 	}
 
 	/**
diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php
index bf4cc6d792..4e50c8ee79 100644
--- a/includes/content/ContentHandler.php
+++ b/includes/content/ContentHandler.php
@@ -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