(bug 42064) Make EditFilterMergedContent more generic.

This is part a the solution to bug 42064: it modifies the EditFilterMergedContent hook
to allow it to be triggered by code outside the EditPage. This is useful when extensions
such as Wikibase implement their own editing mechanism for non-textual content.

Allowing EditFilterMergedContent to be used on non-textual content ensures that
any filters will also be applied to that content.

Note that EditFilterMergedContent was introduced in 1.21 which hasn't been released yet,
so modifying the hooks definition should not be a problem. The only extension that appears
to use the EditFilterMergedContent hook is Translate. I'll supply a patch.

To completely fix bug 42064, I will soon submit two changes to two extensions:

* In the Wikibase extension, trigger the EditFilterMergedContent when changing data
entities.

* In AbuseFilter, make use of the EditFilterMergedContent hook, so filters will also be
applied to non-textual content.

The the dicsussion on bugzilla for the rationale of this architecture.

Change-Id: I99a19c93e99860a91d7f898b0a3fbb72b69baab8

diff --git a/docs/hooks.txt b/docs/hooks.txt
index abf1b80..d04813e 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -856,11 +856,18 @@ $text: content of the edit box
 &$error: error message to return
 $summary: Edit summary for page
 
-'EditFilterMergedContent': Post-section-merge edit filter
-$editor: EditPage instance (object)
-$content: content of the edit box, as a Content object
-&$error: error message to return
+'EditFilterMergedContent': Post-section-merge edit filter.
+This may be triggered by the EditPage or any other facility that modifies page content.
+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.
+$context: object implementing the IContextSource interface.
+$content: content of the edit box, as a Content object.
+$status: Status object to represent errors, etc.
 $summary: Edit summary for page
+$user: the User object representing the user whois performing the edit.
+$minoredit: whether the edit was marked as minor by the user.
 
 'EditFormPreloadText': Allows population of the edit form when creating
 new pages

diff --git a/includes/EditPage.php b/includes/EditPage.php
index cf2b878..0e6eafb 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -1218,6 +1218,54 @@ class EditPage {
                }
        }
 
+       /**
+        * Run hooks that can filter edits just before they get saved.
+        *
+        * @param Content $content the Content to filter.
+        * @param Status  $status for reporting the outcome to the caller
+        * @param User    $user the user performing the edit
+        *
+        * @return bool
+        */
+       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 ) ) ) {
+
+                       # Error messages etc. could be handled within the hook...
+                       $status->fatal( 'hookaborted' );
+                       $status->value = self::AS_HOOK_ERROR;
+                       return false;
+               } elseif ( $this->hookError != '' ) {
+                       # ...or the hook could be expecting us to produce an error
+                       $status->fatal( 'hookaborted' );
+                       $status->value = self::AS_HOOK_ERROR_EXPECTED;
+                       return false;
+               }
+
+               // Run new style post-section-merge edit filter
+               if ( !wfRunHooks( 'EditFilterMergedContent',
+                       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;
+                       return false;
+               } elseif ( !$status->isOK() ) {
+                       # ...or the hook could be expecting us to produce an error
+                       // FIXME this sucks, we should just use the Status object throughout
+                       $this->hookError = $status->getWikiText();
+                       $status->fatal( 'hookaborted' );
+                       $status->value = self::AS_HOOK_ERROR_EXPECTED;
+                       return false;
+               }
+
+               return true;
+       }
+
        /**
         * Attempt submission (no UI)
         *
@@ -1386,17 +1434,7 @@ class EditPage {
                                return $status;
                        }
 
-                       // Run post-section-merge edit filter
-                       if ( !wfRunHooks( 'EditFilterMerged', array( $this, $this->textbox1, &$this->hookError, $this->summary ) ) ) {
-                               # Error messages etc. could be handled within the hook...
-                               $status->fatal( 'hookaborted' );
-                               $status->value = self::AS_HOOK_ERROR;
-                               wfProfileOut( __METHOD__ );
-                               return $status;
-                       } elseif ( $this->hookError != '' ) {
-                               # ...or the hook could be expecting us to produce an error
-                               $status->fatal( 'hookaborted' );
-                               $status->value = self::AS_HOOK_ERROR_EXPECTED;
+                       if ( !$this->runPostMergeFilters( $textbox_content, $status, $wgUser ) ) {
                                wfProfileOut( __METHOD__ );
                                return $status;
                        }
@@ -1510,20 +1548,7 @@ class EditPage {
                                return $status;
                        }
 
-                       // Run post-section-merge edit filter
-                       $hook_args = array( $this, $content, &$this->hookError, $this->summary );
-
-                       if ( !ContentHandler::runLegacyHooks( 'EditFilterMerged', $hook_args )
-                               || !wfRunHooks( 'EditFilterMergedContent', $hook_args ) ) {
-                               # Error messages etc. could be handled within the hook...
-                               $status->fatal( 'hookaborted' );
-                               $status->value = self::AS_HOOK_ERROR;
-                               wfProfileOut( __METHOD__ );
-                               return $status;
-                       } elseif ( $this->hookError != '' ) {
-                               # ...or the hook could be expecting us to produce an error
-                               $status->fatal( 'hookaborted' );
-                               $status->value = self::AS_HOOK_ERROR_EXPECTED;
+                       if ( !$this->runPostMergeFilters( $content, $status, $wgUser ) ) {
                                wfProfileOut( __METHOD__ );
                                return $status;
                        }