r65339 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r65338‎ | r65339 | r65340 >
Date:20:30, 20 April 2010
Author:churchofemacs
Status:resolved (Comments)
Tags:
Comment:
When visiting a 'red link' of a deleted file, a deletion and move log excerpt is provided on the Upload form.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialUpload.php
——@@ -237,6 +237,21 @@
238238 $form->addPreText( wfMsgExt( 'session_fail_preview', 'parseinline' ) );
239239 }
240240
 241+ # Give a notice if the user is uploading a file that has been deleted or moved
 242+ # Note that this is independent from the message 'filewasdeleted' that requires JS
 243+ $desiredTitleObj = Title::newFromText( $this->mDesiredDestName, NS_FILE );
 244+ $delNotice = ''; // empty by default
 245+ if ( !$desiredTitleObj->exists() ) {
 246+ LogEventsList::showLogExtract( &$delNotice, array( 'delete', 'move' ),
 247+ $desiredTitleObj->getPrefixedText(),
 248+ '', array( 'lim' => 10,
 249+ 'conds' => array( "log_action != 'revision'" ),
 250+ 'showIfEmpty' => false,
 251+ 'msgKey' => array( 'upload-recreate-warning' ) )
 252+ );
 253+ }
 254+ $form->addPreText( $delNotice );
 255+
241256 # Add text to form
242257 $form->addPreText( '<div id="uploadtext">' .
243258 wfMsgExt( 'uploadtext', 'parse', array( $this->mDesiredDestName ) ) .
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -2019,6 +2019,9 @@
20202020 'upload_directory_read_only' => 'The upload directory ($1) is not writable by the webserver.',
20212021 'uploaderror' => 'Upload error',
20222022 'upload-summary' => '', # do not translate or duplicate this message to other languages
 2023+'upload-recreate-warning' => "'''Warning: A file by that name has been deleted or moved.'''
 2024+
 2025+The deletion and move log for this page are provided here for convenience:",
20232026 'uploadtext' => "Use the form below to upload files.
20242027 To view or search previously uploaded files go to the [[Special:FileList|list of uploaded files]], (re)uploads are also logged in the [[Special:Log/upload|upload log]], deletions in the [[Special:Log/delete|deletion log]].
20252028
Index: trunk/phase3/RELEASE-NOTES
——@@ -49,6 +49,8 @@
5050 * Special:Listfiles now supports a username parameter
5151 * Special:Random carries over query string parameters
5252 * (bug 23206) Add Special::Search hook for detecting successful "Go"
 53+* When visiting a "red link" of a deleted file, a deletion and move log excerpt
 54+ is provided on the Upload form.
5355
5456 === Bug fixes in 1.17 ===
5557 * (bug 17560) Half-broken deletion moved image files to deletion archive

Follow-up revisions

RevisionCommit summaryAuthorDate
r65356Follow-up r65339: Add new message key to maintenance fileraymond06:37, 21 April 2010
r65372fixing r65339 which broke Upload when file dest was undefinedchurchofemacs10:53, 21 April 2010
r78560Fixing r65339: makeTitleSafe instead of newFromTextchurchofemacs13:57, 18 December 2010
r91594Remove 'upload-wasdeleted' message. Its no longer used....bawolff20:07, 6 July 2011

Comments

#Comment by Church of emacs (talk | contribs)   20:33, 20 April 2010

Just a short comment on this: After I wrote this, I realized that SpecialUpload.php:645 already has some kind of warning for deleted files. However, there are some differences: (a) This requires Javascript. (b) Is is mainly for the process where a user selects a source file and then it gets checked. It doesn't check for wpDestFile in the URL. (c) The JS-warning doesn't actually show a log excerpt, it only links to it.

So while both warnings are more or less about the same, they show up in different scenarios and normally not at the same time: The javascript warning shows up if the user selects a file / file name manually, while the non-js warning (added in this revision) provides a log excerpt when the user clicks on a red File:SomeNonExistingFile.png link.

#Comment by Werdna (talk | contribs)   01:17, 9 December 2010 
+		$desiredTitleObj = Title::newFromText( $this->mDesiredDestName, NS_FILE );

Title::newFromText is the wrong function to use in this case. You should be using Title::makeTitleSafe 

+			LogEventsList::showLogExtract( &$delNotice, array( 'delete', 'move' ),

Call-time pass-by-reference (&$delNotice) is deprecated, and will result in error messages when E_STRICT errors are set to be reported.

#Comment by Church of emacs (talk | contribs)   13:58, 18 December 2010

I'm thinking about making the HTML string the actual return value and ditching number of rows as return value. AFAIK it's only used by CentralAuth/SpecialCentralAuth.php and in this instance it could be easily replaced by setting 'showIfEmpty' => false in the $param array and checking whether the resulting string is empty or not.

This would also make the first parameter (mixed: OutputPage or String-by-reference) obsolete. (I'm not sure whether it should be removed. A lot of extensions seem to depend on the current schema. One could retain the first parameter as optional OutputPage object).

What do you think?

(The other problem has been fixed in r78560.)

#Comment by Reedy (talk | contribs)   02:14, 7 January 2011

FYI: It's not there anymore in core, so i'm guessing that's it :)

Status & tagging log