API edit: allow ConfirmEdit to use the merged parse
authorTim Starling <tstarling@wikimedia.org>
Fri, 5 Dec 2014 04:25:18 +0000 (15:25 +1100)
committerTim Starling <tstarling@wikimedia.org>
Fri, 5 Dec 2014 06:10:20 +0000 (17:10 +1100)
ConfirmEdit was tripling the amount of time it took to save a typical
page via the API, since it was assuming that the APIEditBeforeSave hook
was giving unmerged wikitext, requiring a full parse of the content
before and after the edit in order to check the addurl trigger.
Apparently nobody bothered to update it after Ia59fb6bb.

APIEditBeforeSave is unusable anyway, because it is given the serialized
content, without any information about the content model. It really
needs the Content object in order to do it properly. So instead, adapt
EditFilterMergedContent for the purpose. Incidentally, that avoids the
inelegant defined('MW_API') check in ConfirmEdit's
EditFilterMergedContent handler, since both API and normal edits are
handled by EditFilterMergedContent in the same way.

Unfortunately the interpretation of the 'value' member in the Status
object passed to EditFilterMergedContent is not extensible, being an
integer instead of an associative array. So we add a custom member in
order to get the result array back down the stack.

Another obstacle to this design was the lack of an EditPage object
passed to EditFilterMergedContent. ConfirmEdit was previously directly
calling EditPage::showEditForm() with a $formCallback which outputs the
necessary HTML. Instead, I hacked up runPostMergeFilters() a bit, to
allow the hook to request that the edit page be shown again even if
hookError is not set. Then a new EditPage::showEditForm:fields hook does
the necessary HTML output, instead of $formCallback.

Marked $formCallback as deprecated, since I think it is now unused.

Change-Id: I4b4270dd868a643512d4717927858b6ef0556d8a

docs/hooks.txt
includes/Defines.php
includes/EditPage.php
includes/api/ApiEditPage.php

index 062a0c8..1c6e5c5 100644 (file)
@@ -1048,7 +1048,8 @@ This may be triggered by the EditPage or any other facility that modifies page c
 Use the $status object to indicate whether the edit should be allowed, and to provide
 a reason for disallowing it. Return false to abort the edit, and true to continue.
 Returning true if $status->isOK() returns false means "don't save but continue user
-interaction", e.g. show the edit form.
+interaction", e.g. show the edit form. $status->apiHookResult can be set to an array
+to be returned by api.php action=edit. This is used to deliver captchas.
 $context: object implementing the IContextSource interface.
 $content: content of the edit box, as a Content object.
 $status: Status object to represent errors, etc.
index d0c2226..d63d5ca 100644 (file)
@@ -2,10 +2,6 @@
 /**
  * A few constants that might be needed during LocalSettings.php.
  *
- * Note: these constants must all be resolvable at compile time by HipHop,
- * since this file will not be executed during request startup for a compiled
- * MediaWiki.
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -216,6 +212,7 @@ define( 'MW_SUPPORTS_EDITFILTERMERGED', 1 );
 define( 'MW_SUPPORTS_PARSERFIRSTCALLINIT', 1 );
 define( 'MW_SUPPORTS_LOCALISATIONCACHE', 1 );
 define( 'MW_SUPPORTS_CONTENTHANDLER', 1 );
+define( 'MW_EDITFILTERMERGED_SUPPORTS_API', 1 );
 /**@}*/
 
 /** Support for $wgResourceModules */
index 4a013ef..7b010ea 100644 (file)
@@ -1420,8 +1420,8 @@ class EditPage {
        protected function runPostMergeFilters( Content $content, Status $status, User $user ) {
                // Run old style post-section-merge edit filter
                if ( !ContentHandler::runLegacyHooks( 'EditFilterMerged',
-                       array( $this, $content, &$this->hookError, $this->summary ) ) ) {
-
+                       array( $this, $content, &$this->hookError, $this->summary ) ) )
+               {
                        # Error messages etc. could be handled within the hook...
                        $status->fatal( 'hookaborted' );
                        $status->value = self::AS_HOOK_ERROR;
@@ -1435,14 +1435,23 @@ class EditPage {
 
                // Run new style post-section-merge edit filter
                if ( !wfRunHooks( 'EditFilterMergedContent',
-                       array( $this->mArticle->getContext(), $content, $status, $this->summary,
-                               $user, $this->minoredit ) ) ) {
-
+                               array( $this->mArticle->getContext(), $content, $status, $this->summary,
+                               $user, $this->minoredit ) ) )
+               {
                        # Error messages etc. could be handled within the hook...
-                       // XXX: $status->value may already be something informative...
-                       $this->hookError = $status->getWikiText();
-                       $status->fatal( 'hookaborted' );
-                       $status->value = self::AS_HOOK_ERROR;
+                       if ( $status->isGood() ) {
+                               $status->fatal( 'hookaborted' );
+                               // Not setting $this->hookError here is a hack to allow the hook
+                               // to cause a return to the edit page without $this->hookError
+                               // being set. This is used by ConfirmEdit to display a captcha
+                               // without any error message cruft.
+                       } else {
+                               $this->hookError = $status->getWikiText();
+                       }
+                       // Use the existing $status->value if the hook set it
+                       if ( !$status->value ) {
+                               $status->value = self::AS_HOOK_ERROR;
+                       }
                        return false;
                } elseif ( !$status->isOK() ) {
                        # ...or the hook could be expecting us to produce an error
@@ -2320,6 +2329,9 @@ class EditPage {
         * Send the edit form and related headers to $wgOut
         * @param callable|null $formCallback That takes an OutputPage parameter; will be called
         *     during form output near the top, for captchas and the like.
+        *
+        * The $formCallback parameter is deprecated since MediaWiki 1.25. Please
+        * use the EditPage::showEditForm:fields hook instead.
         */
        function showEditForm( $formCallback = null ) {
                global $wgOut, $wgUser;
@@ -2378,6 +2390,7 @@ class EditPage {
                ) );
 
                if ( is_callable( $formCallback ) ) {
+                       wfWarn( 'The $formCallback parameter to ' . __METHOD__ . 'is deprecated' );
                        call_user_func_array( $formCallback, array( &$wgOut ) );
                }
 
index c1598c8..7f4b222 100644 (file)
@@ -406,7 +406,14 @@ class ApiEditPage extends ApiBase {
                switch ( $status->value ) {
                        case EditPage::AS_HOOK_ERROR:
                        case EditPage::AS_HOOK_ERROR_EXPECTED:
-                               $this->dieUsageMsg( 'hookaborted' );
+                               if ( isset( $status->apiHookResult ) ) {
+                                       $r = $status->apiHookResult;
+                                       $r['result'] = 'Failure';
+                                       $apiResult->addValue( null, $this->getModuleName(), $r );
+                                       return;
+                               } else {
+                                       $this->dieUsageMsg( 'hookaborted' );
+                               }
 
                        case EditPage::AS_PARSE_ERROR:
                                $this->dieUsage( $status->getMessage(), 'parseerror' );