Last modified: 2010-05-15 15:37:37 UTC
The following bug is probably specific to Firefox 1.0.4. In [[Special:Upload]], if the "Destination filename" is already filled in and I choose "Browse" and select a file, both the "Source filename" and "Destination filename" get overwritten. This bug makes the "Upload a new version" feature useless.
Actually, I don't think it's specific at all - it's a design flaw in some natty JavaScript: filling in a source filename calls a function to fill the destination filename, even if it's already filled. Normally, this is handy, but as you say it breaks the "upload new version" trick. A hacky but possibly reasonable workaround would be to check for "wpDestFile" in the URL, and not overwrite the value in the form if that's where it came from. Patch to do that follows.
Created attachment 780 [details] workaround by checking for "wpDestFile" in the URL [Note that I've tested this hack on exactly one browser (Mozilla 1.7.8), so it may not work anywhere else, for all I know.]
I think the behavior should be: automatically fill in the "Destination filename" only if it is blank. That way, if the user fills the Destination field ''first'', then selects their file, the Destination won't get wiped out. You can't lose information by replacing a blank Destination field with the Source file name, since that's what happens anyway at upload.
Created attachment 782 [details] better implementation of same idea - don't attach the JavaScript to the form if DestFile pre-determined This patch is a lot less hacky, because it changes the actual HTML, not the JavaScript.
Created attachment 783 [details] as above, but also toggle the auto-filler if the destination name is manually editted/cleared Hm, I think over-writing it even when it's full is deliberate - it might just have been auto-filled already, and the user hasn't touched it; in that situation, we don't want it to be named after the first file the user selected if they then changed their mind. This patch includes the previous one, but also adds an event handler to the *destination* box, so that if you manually type a filename, the auto-filler is disabled. It's done as a toggle, though, so if you clear the dest filename (or replace it with a single space), the auto-filler is switched on, even if you originally used "upload new version" to pre-fill it. Again, only tested under Moz-1.7.8 so far, so may break in other browsers - for instance, the code assumes that the autofiller doesn't itself trigger the change handler for the destination field...
(In reply to comment #5) Good point about the user selecting an input file, changing their mind, and selecting a different file. We certainly don't want the old input filename to remain as the destination name. The behavior you describe is now fairly complicated. What is the purpose of filling in the destination field at all? The ONLY benefit, as far as I can see, is in the case where User wants the destination filename to be a modification of the source filename. In that case, they're saved the step of copying the source filename to the destination field before modifying it. Perhaps it's better to simply never touch the destination field? [[Principle of Least Astonishment]]?
This seems to be a side-effect of the fix for bug 2527.
(In reply to comment #6) > The behavior you describe is now fairly complicated. What is the purpose of > filling in the destination field at all? I think the idea is that, since the destination field exists, having it blank may not be the "least astonishment" anyway - users may not be familiar with the concept of "default values", and thus won't realise that leaving the destination name blank is the same as typing in the name of the source file; so the JavaScript saves them thinking they need to type something in. And, as you say, they can modify the name, so if it's "Foobar 3.jpeg", they can spot it and go "oh, it can just be Foobar.jpeg".
To clarify (and justify), from the user's point of view my patch creates the following behaviour: 1) "normally", when the user selects a file, the destination filename is filled in automatically; this saves us having to explain that that box is optional (and what it would default to), and allows the user to edit the name 2) if the user selects a second file, the dest. name is filled a second time; this makes sense because the name already in the box is just the JavaScript's "best guess" from last time 3) if the user has manually typed in a name, or manually editted it in any way, it is not over-written; this makes sense because they have conciously over-ridden the JavaScript's best guess 4) if the dest.name is filled automatically by an "upload a new version" or "image not found" link, that name is not over-written; not doing this destroys the point of those links 5) if the user manually *blanks* the dest.name (either completely or with the spacebar), it is considered ready to fill; this makes sense, because the form will now look like it did in (1), and so should perform the same The only problem with all this is that there's no feedback of whether the name will be auto-filled or not; perhaps a check-box could be added that reflects (and/or can override) the current state? If we don't want any of this complexity, a completely alternative approach would be to display the automatically generated name *non-editable* in a label like "(will be called '<name>' if left blank)". This label could be generated and updated through JavaScript (it would need to not be there at all without JS), so that it would essentially behave like the current one, but the user would have to copy-and-paste to make minor alterations. This reflects the fact that the whole JavaScript is only sugar on top of code which can handle a blank destination name fine.
(In reply to comment #9) > "(will be called '<name>' if left blank)". This label could be generated and > updated through JavaScript (it would need to not be there at all without JS), I really like this idea.
Making the destination non-editable in the 'upload new version' mode (read-only and disabled, so consistent with standard GUI conventions) would I think be the most sensible thing.
It's been 3 weeks. Could someone please apply a fix of some sort? Any fix would be better than no fix at all. As it is, the "Upload new version" feature is a booby trap.
I've committed the patch in attachment 782 [details] (which just doesn't attach the JS event handler if the destname is already known) to CVS HEAD and the 1.5 branch as a "hotfix" for this. Brion's probably right that the 'upload new version' mode should have a disabled destname box, but there are other things to tweak at the same time to make this feel like the form is in a different mode, such as a message telling the user that that's what they're doing, and perhaps a different (or no) over-write warning when they submit it.
I've done a quick hack of my own monobook.js on Commons because I'm sick of this. It's one extra line of javascript which returns if the destination filename is not empty.
So is this issue resolved or...?
(In reply to comment #15) > So is this issue resolved or...? The issue has been improved, but not really fixed. In [[Special:Upload]], enter a destination filename, then select the source filename. The destination you entered is overwritten.
Original issue is long fixed... Have gone ahead and integrated a variant of the patch to cover the secondary issue noted in comment #16. Whether to use the autofilled name or not is now controlled via a boolean toggle, which is initialized depending on whether a dest filename was provided for the form. It's flipped when the destination filename is manually edited -- off if non-empty, or back on if empty. r32001 on trunk for 1.13.
Brion, re r1=31761&r2=32001&pathrev=32001">http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/SpecialUpload.php?r1=31761&r2=32001&pathrev=32001 lines 1075/1077 (the "onchange" handler on the wpDestFile field): Note that neither IE6 nor FF2.0.0.12 fire onchange when the wpDestFile field is set by the user through a selection from the autocompletion suggestions dropdown. IE6 fires onpropertychanged in this case, FF2 does nothing at all. FF3 (and, if I understand their comments correctly, also FF2.0.0.13) should indeed fire onchange. For FF, see https://bugzilla.mozilla.org/show_bug.cgi?id=388558 For IE, see http://msdn2.microsoft.com/en-us/library/ms533032.aspx and http://msdn2.microsoft.com/en-us/library/ms536956.aspx Also note that on IE, onpropertychange is also fired on keypresses. In the onpropertychange handler, a check for event.propertyName == 'value' can be used to detect changes to the value of the field. P.S.: sorry if I did something wrong by re-opening this.
This has been reported as working on IE 6 currently; if problems can be reproduced with current code, reopen with details.