Page MenuHomePhabricator

Show warning on redirect creation if there are other redirects pointing to it (avoid double redirects)
Open, LowPublicFeature

Description

Steps to replicate the issue:

  • Create sandbox pages at [[Bar]] and [[Baz]].
  • Create a redirect at [[Foo]] to [[Bar]] .
  • Try to change [[Bar]] into a redirect to [[Baz]].
  • This is permitted, with no error thrown.
  • There is now a double redirect: [[Foo]][[Bar]][[Baz]].

Feature summary:

On redirect creation, Whatlinkshere should be checked for redirects to the page. If there are any, there should be an error message, ideally which lists all of those redirects, so they can be fixed.

A setting T396214: Config setting `$wgDisableDoubleRedirectCreationErrors` to control throwing of double redirect creation errors $wgDisableDoubleRedirectCreationErrors could disable this on big wikis where there are fixer-bots.

As per T384893, if the redirect page to this page is also the redirect target, it will now throw an appropriate error. So a redirect to the edited page should be deliberately ignored if it's also the redirect target.

Benefits:

Would discourage double redirect creation. Although the big WMF wikis have bots to fix these, many smaller wikis don't.

Notes

Existing edit constraints are located in includes / editpage / Constraint

This situation may occur independently of some other errors. So, the error should be thrown separately to any other errors that are detected.

It might be a good idea to be able to turn this feature off with a configuration setting, as it may not be needed on wikis with fixer-bots, but probably is needed on smaller independent 3rd party wikis.

The error message should look something like this:

You are trying to create a redirect, but the following {{plural|{{{1}}}|page is already a redirect|{{{1}}} pages are already redirects}} to this page.  Clicking 'save' will therefore create {{plural|{{{1}}}|a double redirect from this page|double redirects from these pages}}:

<span class="list of double redirects to be created by this edit">
# [https://sitename.com/wiki/{{{2}}}&redirect=no]
# [https://sitename.com/wiki/{{{3}}}&redirect=no]
</span>

The list of redirects probably should max out at some arbitrary limit.
The list of redirects should also use &redirect=no which means using the long Canononical url form: e.g. https://en.wikipedia.org/w/index.php?title=USA&redirect=no

That message probably should be enough for any JavaScript script/gadget to use as input and fix the offending redirects. But that's a different development.

Add to includes / editpage / EditPage.php a single line, alphabetically in the right place:

use MediaWiki\EditPage\Constraint\IncomingRedirectDoubleRedirectConstraint;

AI code

AI suggests the following code:

<?php
/**
 * Prevent creating double redirects by checking incoming redirects to the page.
 *
 * 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
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, write to the Free Software Foundation, Inc.,
 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 * http://www.gnu.org/copyleft/gpl.html
 *
 * @file
 */

namespace MediaWiki\EditPage\Constraint;

use MediaWiki\Content\Content;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Title\Title;
use MediaWiki\Status\StatusValue;
use Wikimedia\Rdbms\IDatabase;

class IncomingRedirectDoubleRedirectConstraint implements IEditConstraint
{
    private Content $newContent;
    private LinkTarget $title;
    private IDatabase $db;
    private string $result = '';
    /** @var Title[] List of incoming redirect titles */
    private array $incomingRedirects = [];
    private ?LinkTarget $newRedirectTarget = null;

    /**
     * @param Content $newContent The new content of the page being edited
     * @param LinkTarget $title The title of the page being edited
     * @param IDatabase $db Database connection for querying redirects
     */
    public function __construct(Content $newContent, LinkTarget $title, IDatabase $db)
    {
        $this->newContent = $newContent;
        $this->title = $title;
        $this->db = $db;
        $this->newRedirectTarget = $newContent->getRedirectTarget();
    }

    /**
     * Check if the new edit would create a double redirect by looking for incoming redirects.
     *
     * @return string One of self::CONSTRAINT_PASSED or self::CONSTRAINT_FAILED
     */
    public function checkConstraint(): string
    {
        // Only check if the new content is a redirect
        if ($this->newRedirectTarget === null) {
            $this->result = self::CONSTRAINT_PASSED;
            return self::CONSTRAINT_PASSED;
        }

        // Query for pages that redirect to this page (incoming redirects)
        $conds = [
            'rd_namespace' => $this->title->getNamespace(),
            'rd_title' => $this->title->getDBkey(),
            'rd_interwiki' => '',
        ];

        $res = $this->db->newSelectQueryBuilder()
            ->select(['rd_from'])
            ->from('redirect')
            ->where($conds)
            ->caller(__METHOD__)
            ->fetchResultSet();

        $incomingRedirects = [];
        foreach ($res as $row) {
            // Convert page ID (rd_from) to Title object
            $redirectingTitle = Title::newFromID($row->rd_from);
            if ($redirectingTitle !== null) {
                $incomingRedirects[] = $redirectingTitle;
            }
        }

        if (count($incomingRedirects) > 0) {
            $this->incomingRedirects = $incomingRedirects;
            $this->result = self::CONSTRAINT_FAILED;
            return self::CONSTRAINT_FAILED;
        }

        $this->result = self::CONSTRAINT_PASSED;
        return self::CONSTRAINT_PASSED;
    }

    /**
     * Return a StatusValue describing the constraint result for legacy error handling.
     *
     * @return StatusValue
     */
    public function getLegacyStatus(): StatusValue
    {
        if ($this->result === self::CONSTRAINT_FAILED) {
            $status = StatusValue::newFatal('edit-constraint-incoming-double-redirect');

            // Format the list of incoming redirects as a comma-separated string
            $titlesText = array_map(
                fn(Title $title) => $title->getPrefixedText(),
                $this->incomingRedirects
            );

            $listText = implode(', ', $titlesText);

            // Add the list to the status details for the message
            $status->params = [$listText];

            return $status;
        }
        return StatusValue::newGood();
    }
}

Event Timeline

  • Create a redirect to [[Foo]]from [[Bar]].

To clarify, did you mean "create a redirect from [[Bar]] to [[Foo]]” — which would (at the end of the steps) result in [[Bar]] being a double redirect ([[Bar]] -> [[Foo]] -> [[Baz]])?
(ignore this, i misread what you’d typed, and only realised it after submitting the comment!)


To comment on the feature request, I’m not sure if there should be an error message displayed for attempting to blank-and-redirect a page that has redirects to it — I feel like that might imply that the end user is attempting to do something that they potentially shouldn’t be doing, when (e.g.) blanking-and-redirecting one page to another feels like it should be a normal/acceptable edit from the software’s point-of-view. I also feel like a message like this might shift a maintenance burden onto an editor that the wiki/the software should potentially arguably be doing by itself without editors needing to worry about it/consider it, or that could be expected to be done by editors who patrol that wiki’s Special:DoubleRedirects report.

If a message like this is added, though, I feel like its appearance should maybe be controlled by a config variable (potentially set to false by default), so that wikis whose editors don’t need to worry about double redirects (e.g., wikis with redirect-fixing bots and/or that don’t want their editors to see this message) don’t have to have it displayed.

Pppery subscribed.

Agreed with A smart kitten here.

Thanks for the feedback @A_smart_kitten.

I've tried to make the explanation as logical as it is now.

Because you edited, I'm not quite sure what @ppery is agreeing with - your first statement or your edit.

Anyway, I think a variable as you suggest may be a good idea. It could control both this and the double redirect checker (T326056). With respect, I don't really follow your other line of reasoning, though. I think MW should throw errors on double redirect creation, however this occurs, if there aren't any bots that will tidy up the mess.

I meant A smart kitten's second paragraph.

I think the objections to this should be alleviated by T396214: Config setting `$wgDisableDoubleRedirectCreationErrors` to control throwing of double redirect creation errors, which could be turned on on wikis where there are fixer-bots to fix double redirects.