EditPage: Disentangle edit summary and section title

Previously, `$this->summary` was used for two different purposes.
Usually it was just the summary. But when `$this->section` was 'new',
then it was actually the section title most of the time – unless
`$this->sectiontitle` was also set (in which case it took priority),
and until it was replaced by the real edit summary (near the end of
the processing, after copying the section title to the page content
and before saving changes).

Unsurprisingly some of the code didn't handle this duality correctly,
causing T191722 and T311533.

Now `$this->summary` is always the summary, and when `$this->section`
is 'new', then `$this->sectiontitle` is always the new section title.

The only place where this duality remains is in the input attributes
and query parameters, where 'wpSummary' is still used for both the
summary and the section title inputs (only one of them can appear,
depending on whether `$this->section` is 'new'). It would be an
unreasonable backwards-compatibility break to change this, and the
code handling this is somewhat isolated from the rest of the logic.

Bug: T191722
Bug: T311533
Change-Id: I5313ca9a045d112ece390b011a34192220e2abc1

1 files changed, 4 insertions, 22 deletions

diff --git a/includes/editpage/Constraint/SpamRegexConstraint.php b/includes/editpage/Constraint/SpamRegexConstraint.php
index 4b279282134c..f3a735ccf10d 100644
--- a/includes/editpage/Constraint/SpamRegexConstraint.php
+++ b/includes/editpage/Constraint/SpamRegexConstraint.php
@@ -43,7 +43,7 @@ class SpamRegexConstraint implements IEditConstraint {
 	/** @var string */
 	private $summary;
 
-	/** @var string */
+	/** @var ?string */
 	private $sectionHeading;
 
 	/** @var string */
@@ -62,7 +62,6 @@ class SpamRegexConstraint implements IEditConstraint {
 	 * @param LoggerInterface $logger for logging hits
 	 * @param SpamChecker $spamChecker
 	 * @param string $summary
-	 * @param string $section
 	 * @param ?string $sectionHeading
 	 * @param string $text
 	 * @param string $reqIP for logging hits
@@ -72,28 +71,15 @@ class SpamRegexConstraint implements IEditConstraint {
 		LoggerInterface $logger,
 		SpamChecker $spamChecker,
 		string $summary,
-		string $section,
 		?string $sectionHeading,
 		string $text,
 		string $reqIP,
 		Title $title
 	) {
-		if ( $section == 'new' ) {
-			// $wgSpamRegex is enforced on this new heading/summary because, unlike
-			// regular summaries, it is added to the actual wikitext.
-			// sectiontitle is only set if the API is used with `sectiontitle`, otherwise
-			// the summary is used which comes from the API `summary` parameter or the
-			// "Add Topic" user interface
-			$sectionHeadingToCheck = $sectionHeading ?? $summary;
-		} else {
-			// No section heading to check
-			$sectionHeadingToCheck = '';
-		}
-
 		$this->logger = $logger;
 		$this->spamChecker = $spamChecker;
 		$this->summary = $summary;
-		$this->sectionHeading = $sectionHeadingToCheck;
+		$this->sectionHeading = $sectionHeading;
 		$this->text = $text;
 		$this->reqIP = $reqIP;
 		$this->title = $title;
@@ -101,12 +87,8 @@ class SpamRegexConstraint implements IEditConstraint {
 
 	public function checkConstraint(): string {
 		$match = $this->spamChecker->checkSummary( $this->summary );
-		if ( $match === false ) {
-			// $wgSpamRegex is enforced on this new heading/summary because, unlike
-			// regular summaries, it is added to the actual wikitext.
-			// EditPage has already determined, based on if this is the API with `sectiontitle`,
-			// or action=edit, or the API with `summary`, what will be the section title.
-			// If the section isn't new, the $this->sectionHeading is an empty string
+		if ( $match === false && $this->sectionHeading !== null ) {
+			// If the section isn't new, the $this->sectionHeading is null
 			$match = $this->spamChecker->checkContent( $this->sectionHeading );
 		}
 		if ( $match === false ) {