Make errors from ArticleSave hooks propagate to the interface

Aborting a save from the ArticleSave hook and putting an error in
$status didn't actually propagate the error message to the user, but
instead displayed the edit conflict page (!). Fix this so that if we get
an unrecognized error from ArticleSave, we treat it as an extension
error and render it rather than going into conflict mode.

Similarly, make the API attempt to render the error through
dieUsageMsg() like it already does for AS_END

Change-Id: Iccf78480240d0c7ed321438c8190472805957099

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 5979ed4..c84ae75 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -968,7 +968,6 @@ class EditPage {
                $bot = $wgUser->isAllowed( 'bot' ) && $this->bot;
                $status = $this->internalAttemptSave( $resultDetails, $bot );
                // FIXME: once the interface for internalAttemptSave() is made nicer, this should use the message in $status
-
                if ( $status->value == self::AS_SUCCESS_UPDATE || $status->value == self::AS_SUCCESS_NEW_ARTICLE ) {
                        $this->didSave = true;
                }
@@ -1039,6 +1038,14 @@ class EditPage {
                                $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage';
                                throw new PermissionsError( $permission );
 
+                       default:
+                               // We don't recognize $status->value. The only way that can happen
+                               // is if an extension hook aborted from inside ArticleSave.
+                               // Render the status object into $this->hookError
+                               // FIXME this sucks, we should just use the Status object throughout
+                               $this->hookError = '<div class="error">' . $status->getWikitext() .
+                                       '</div>';
+                               return true;
                }
                return false;
        }
@@ -1427,8 +1434,17 @@ class EditPage {
                        wfProfileOut( __METHOD__ );
                        return $status;
                } else {
-                       $this->isConflict = true;
-                       $doEditStatus->value = self::AS_END; // Destroys data doEdit() put in $status->value but who cares
+                       // Failure from doEdit()
+                       // Show the edit conflict page for certain recognized errors from doEdit(),
+                       // but don't show it for errors from extension hooks
+                       $errors = $doEditStatus->getErrorsArray();
+                       if ( in_array( $errors[0][0], array( 'edit-gone-missing', 'edit-conflict',
+                               'edit-already-exists' ) ) )
+                       {
+                               $this->isConflict = true;
+                               // Destroys data doEdit() put in $status->value but who cares
+                               $doEditStatus->value = self::AS_END;
+                       }
                        wfProfileOut( __METHOD__ );
                        return $doEditStatus;
                }

diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php
index 19b1950..0b7ac41 100644 (file)
--- a/includes/api/ApiEditPage.php
+++ b/includes/api/ApiEditPage.php
@@ -340,16 +340,11 @@ class ApiEditPage extends ApiBase {
                                $this->dieUsageMsg( 'summaryrequired' );
 
                        case EditPage::AS_END:
+                       default:
                                // $status came from WikiPage::doEdit()
                                $errors = $status->getErrorsArray();
                                $this->dieUsageMsg( $errors[0] ); // TODO: Add new errors to message map
                                break;
-                       default:
-                               if ( is_string( $status->value ) && strlen( $status->value ) ) {
-                                       $this->dieUsage( "An unknown return value was returned by Editpage. The code returned was \"{$status->value}\"" , $status->value );
-                               } else {
-                                       $this->dieUsageMsg( array( 'unknownerror', $status->value ) );
-                               }
                }
                $apiResult->addValue( null, $this->getModuleName(), $r );
        }