Page MenuHomePhabricator

Deleting redirects in file: namespace tries to delete the redirect target, not the redirect
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

https://commons.wikimedia.org/w/index.php?title=special:diff/606409492/606450401

Reported

What happens?:

delete of file redirect at Commons (430ร—969 px, 12 KB)

  • The deletion form includes the title of the redirect target, not the redirect page
  • If submitted, the redirect target is deleted, not the redirect page

What should have happened instead?:

It should be identifying the redirect, not the redirect target

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:

WMF current version (1.38.0-wmf.7)

As a note. Using something like AWB is able to delete properly, so it would seem that the API is unaffected.

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptNov 10 2021, 2:51 AM
AntiCompositeNumber added a subscriber: Daimona.

Appears to have been caused back in October (1.38.0-wmf.4) with rMW23ec8052ce39: Final sync of DeleteAction and FileDeleteAction forms when the link was added. Looks like this was fixed in master already by rMW6cbff5bbac2f: Make DeleteAction and FileDeleteAction share showForm, both as part of T288282: Build the page delete UI from DeleteAction, not Article. Given that there's no train this week, a backport would probably be warranted (there are no deploys on Thursday, but Wednesday is open).

Daimona set the point value for this task to 2.

Thanks for reporting this. Looking at the changes to DeleteAction, this was actually caused by r711477: FileDeleteAction has a check for redirect files (and also non-local and non-existing files) which triggers a normal article deletion. This is currently happening in FileDeleteAction::tempDelete by calling parent::tempDelete, but that's not enough: everything is still happening inside the FileDeleteAction instance, hence any call to a local method (i.e. $this->foo()`) will use the method in FileDeleteAction (and not DeleteAction) if it exists. r712747 does make it worse by showing the message, and r713299 does fix it by reducing inherited methods, but the underlying problem still exists and is waiting for an innocent code change to trigger similar bugs again. So I'm going to do the switch in WikiFilePage instead.

Change 737917 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Switch between file deletion and normal deletion in WikiFilePage

https://gerrit.wikimedia.org/r/737917

Change 737917 merged by jenkins-bot:

[mediawiki/core@master] Switch between file deletion and normal deletion in WikiFilePage

https://gerrit.wikimedia.org/r/737917

dom_walden subscribed.

I can delete/suppress a redirect to a file without touching the redirect target.

I also checked that the file delete form is the same as it was before we started working on this wish (including its limitations). For example:

  • The delete reasons correspond to file deletion, not regular article deletion
    file_delete_reasons.png (529ร—994 px, 87 KB)
  • No warning if other pages link to the file/file redirect (in contrast to when deleting a regular article)
  • No warning if file/file redirect has subpages (in contrast to when deleting a regular article)

I also tested permissions (including the delete, bigdelete and suppressrevision).

Test environment: Various beta wikis.

I also checked that the file delete form is the same as it was before we started working on this wish (including its limitations).

Thanks for the due diligence and for back-functional comparison @dom_walden and thanks for the functionality @Daimona