Wikimedia Bugzilla is closed!

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.