From 278e195ab7ce653105db4f8a44ca5f70c2e3afa7 Mon Sep 17 00:00:00 2001
From: Func <Funcer@outlook.com>
Date: Wed, 17 Feb 2021 04:11:46 +0000
Subject: [PATCH] Set $status->value in
 EditFilterMergedContentHookConstraint::checkConstraint() properly to display
 error message

When hook handler return false and use $status->fatal() to put up a error,
we should set $status->value to self::AS_HOOK_ERROR_EXPECTED to show edit form
and display error message.

Bug: T273354
Change-Id: I02d643c5cb1978da8ab749856493d75137b6cb02
---
 RELEASE-NOTES-1.37                                            | 2 ++
 .../Constraint/EditFilterMergedContentHookConstraint.php      | 3 ++-
 tests/phpunit/includes/EditPageConstraintsTest.php            | 4 ++--
 .../Constraint/EditFilterMergedContentHookConstraintTest.php  | 4 ++--
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37
index 69ab08b2aa0..52fdbb74af6 100644
--- a/RELEASE-NOTES-1.37
+++ b/RELEASE-NOTES-1.37
@@ -81,6 +81,8 @@ because of Phabricator reports.
    - findVariantLink()
    - mConverter
    - updateConversionTable()
+* (T273354) When an edit is prevented by an 'EditFilterMergedContent' hook
+  handler without changing the status, the edit form will now be displayed.
 
 === Deprecations in 1.37 ===
 * JobQueue::getWiki, deprecated in 1.33, now emits deprecation warnings.
diff --git a/includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php b/includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php
index e60e4972f11..67bcb8b3cfe 100644
--- a/includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php
+++ b/includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php
@@ -102,7 +102,8 @@ class EditFilterMergedContentHookConstraint implements IEditConstraint {
 			}
 			// Use the existing $status->value if the hook set it
 			if ( !$this->status->value ) {
-				$this->status->value = self::AS_HOOK_ERROR;
+				// T273354: Should be AS_HOOK_ERROR_EXPECTED to display error message
+				$this->status->value = self::AS_HOOK_ERROR_EXPECTED;
 			}
 			return self::CONSTRAINT_FAILED;
 		}
diff --git a/tests/phpunit/includes/EditPageConstraintsTest.php b/tests/phpunit/includes/EditPageConstraintsTest.php
index 28da40304ee..a4790e92bf5 100644
--- a/tests/phpunit/includes/EditPageConstraintsTest.php
+++ b/tests/phpunit/includes/EditPageConstraintsTest.php
@@ -376,13 +376,13 @@ class EditPageConstraintsTest extends MediaWikiLangTestCase {
 
 	public function provideTestEditFilterMergedContentHookConstraint() {
 		yield 'Hook returns false, status is good, no value set' => [
-			false, null, false, EditPage::AS_HOOK_ERROR, 'AS_HOOK_ERROR'
+			false, null, false, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'
 		];
 		yield 'Hook returns false, status is good, value set' => [
 			false, 1234567, false, 1234567, 'custom value 1234567'
 		];
 		yield 'Hook returns false, status is not good' => [
-			false, null, true, EditPage::AS_HOOK_ERROR, 'AS_HOOK_ERROR'
+			false, null, true, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'
 		];
 		yield 'Hook returns true, status is not ok' => [
 			true, null, true, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'
diff --git a/tests/phpunit/unit/includes/editpage/Constraint/EditFilterMergedContentHookConstraintTest.php b/tests/phpunit/unit/includes/editpage/Constraint/EditFilterMergedContentHookConstraintTest.php
index 94940a6f80a..446e2f7e893 100644
--- a/tests/phpunit/unit/includes/editpage/Constraint/EditFilterMergedContentHookConstraintTest.php
+++ b/tests/phpunit/unit/includes/editpage/Constraint/EditFilterMergedContentHookConstraintTest.php
@@ -66,9 +66,9 @@ class EditFilterMergedContentHookConstraintTest extends MediaWikiUnitTestCase {
 
 	public function testFailure_goodStatus() {
 		// Code path 1: Hook returns false, but status is still good
-		// Status has no value set, falls back to AS_HOOK_ERROR
+		// Status has no value set, falls back to AS_HOOK_ERROR_EXPECTED
 		$constraint = $this->getConstraint( false );
-		$this->assertConstraintFailed( $constraint, IEditConstraint::AS_HOOK_ERROR );
+		$this->assertConstraintFailed( $constraint, IEditConstraint::AS_HOOK_ERROR_EXPECTED );
 	}
 
 	public function testFailure_badStatus() {