Page MenuHomePhabricator

Wrong error message when undo fails because of a merge conflict
Closed, ResolvedPublicBUG REPORT

Description

Originally reported at enwiki Teahouse.

Steps to replicate the issue (include links if applicable):

Visit https://en.wikipedia.org/w/index.php?title=User:Suffusion_of_Yellow/sandbox2&action=edit&undoafter=1020913085&undo=1068711297

What happens?:

I see "The edit could not be undone because it does not exist or was deleted. " (undo-norev). However, none of the revisions involved are deleted. It's just a simple merge conflict.

What should have happened instead?:

I see "The edit could not be undone due to conflicting intermediate edits; if you wish to undo the change, it must be done manually." (enwik's version of undo-failure)

Other information (browser name/version, screenshots, etc.):

Revision 540bddfb1fb3ec recently did something with the error messages; perhaps it's related?

Event Timeline

Indeed, this was changed here (below)

diff --git a/includes/EditPage.php b/includes/EditPage.php
index a033994..b36a659 100644
--- a/includes/EditPage.php
+++ b/includes/EditPage.php

@@ -1518,7 +1518,7 @@
 
  if ( $content === false ) {
 	# Warn the user that something went wrong
-	$undoMsg = 'failure';
+	$undoMsg = 'norev';
  }

Change 867278 had a related patch set uploaded (by Samtar; author: Samtar):

[mediawiki/core@master] EditPage.php: Use `undo-failure` instead of `undo-norev`

https://gerrit.wikimedia.org/r/867278

Change 867278 had a related patch set uploaded (by Samtar; author: Samtar):

[mediawiki/core@master] EditPage.php: Use `undo-failure` instead of `undo-norev`

https://gerrit.wikimedia.org/r/867278

For what it's worth, I'm unsure which message makes more sense — here's a patch for switching back though, should that be desired.

If the revision fails to load (i.e. not a merge conflict) then the same case is hit. That's what I was testing when I changed the message. We should try to provide the correct message rather than just guess based on whatever is more common.

Change 867278 abandoned by Samtar:

[mediawiki/core@master] EditPage.php: Use `undo-failure` instead of `undo-norev`

Reason:

T325019#8462051

https://gerrit.wikimedia.org/r/867278

Easy to make it work. Giving up on testing it with PHPUnit for now. There's zero test coverage of EditPage::edit() and EditPage::getContentObject().

Change 867293 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Use more specific error message on undo failure

https://gerrit.wikimedia.org/r/867293

Change 867293 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Use more specific error message on undo failure

https://gerrit.wikimedia.org/r/867293