Page MenuHomePhabricator

Upload file form does not update file name if the provided file name is not valid
Open, Needs TriagePublicBUG REPORT

Description

I think this is a bug since to me it is not the expected behaviour.

For a uploadable field, in the upload window, if the file name provided in the "Destination filename" field is not a valid file name and the user clicks on upload, a warning appears saying that the file name has been changed to a new valid name. However, the name of the file in the "Destination filename" field is not updated, if the user clicks on the button "Ignore warning and wave file anyway", the file is saved, but the file name returned is wrong.

Compare this to the MediaWiki Special:Upload page, here the file name in the "Destination filename" field it is updated if a user enters an invalid name.

I spent quite a lot of time looking through the code trying to see what the problem was, but I still don't understant how the specific code for the upload window is working. The files for the upload form are PF_UploadWindow.php and PF_UploadForm.php, which are based on the MediWiki files SpecialUpload.php and UploadForm.php. In SpecialUpload.php, in the lines 396-398, the following code seems to be the one responsible for updating the file name field if the inserted name is not valid:

if ( $warning == 'badfilename' ) {
    $this->mDesiredDestName = Title::makeTitle( NS_FILE, $args )->getText();
}

I tried copying this code to the PF_UploadWindow.php file, but it doesn't work.

My setup is:

MediaWiki 1.32.0
PHP 7.2.19
Page Forms 4.6

Event Timeline

Tombolano created this task.Oct 3 2019, 5:37 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 3 2019, 5:37 PM

I have made a patch to solve the issue.

The problem is that when the form object is constructed in PF_UploadWindow, the destination filename parameter is always ignored and the final filename is the one from the web request. This is due to how the destination filename field is defined in PF_UploadForm.php, the definition is the following:

array(
'DestFile' => array(
	'type' => 'text',
	'section' => 'description',
	'id' => 'wpDestFile',
	'label-message' => 'destfilename',
	'size' => 60,
)

In UploadForm.php the field definition has 2 aditional fields, 'default', and 'nodata', the definition is the following:

'DestFile' => [
	'type' => 'text',
	'section' => 'description',
	'id' => 'wpDestFile',
	'label-message' => 'destfilename',
	'size' => 60,
	'default' => $this->mDestFile,
	# @todo FIXME: Hack to work around poor handling of the 'default' option in HTMLForm
	'nodata' => strval( $this->mDestFile ) !== '',
]

With this field definition the filename passed to the form class is no longer ignored

The patch consist in copying the above definition of the destination file from UploadForm.php to PF_UploadForm.php, and copying the following code from SpecialUpload.php to PF_UploadWindow.php:

if ( $warning == 'badfilename' ) {
	$this->mDesiredDestName = Title::makeTitle( NS_FILE, $args )->getText();
}

Change 541232 had a related patch set uploaded (by Tombolano; owner: Tombolano):
[mediawiki/extensions/PageForms@master] Fix for renaming filename on badfilename warning in the upload window

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