Last modified: 2013-05-28 14:30:37 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T41852, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 39852 - [Regression] "Copy metadata" link not always at the 1st image
[Regression] "Copy metadata" link not always at the 1st image
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UploadWizard (Other open bugs)
master
All All
: Unprioritized major (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-31 21:02 UTC by Raimond Spekking
Modified: 2013-05-28 14:30 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Raimond Spekking 2012-08-31 21:02:36 UTC
The "Copy metadata" functionality is lost after the code update on 30. August 2012.
Comment 1 Raimond Spekking 2012-08-31 21:06:13 UTC
Rephrase:

I found the link now in a batch upoad of 16 images: The "copy metadata" link is now at the 4th image of the batch. I guess the 4th postion is random?
Comment 2 Mark Holmquist 2012-08-31 21:21:06 UTC
I don't think this is a regression, or at least, I think the regression is caused by us assuming reasonable treatment of upload order. See also bug 39746.
Comment 3 Erik Moeller 2012-08-31 21:35:01 UTC
It's a regression in that the upload order is now broken again, causing this functionality to fail as well.
Comment 4 Erik Moeller 2012-08-31 21:36:44 UTC
See also https://gerrit.wikimedia.org/r/#/c/4987/ which was the change I made a while ago to fix the display order.
Comment 5 Mark Holmquist 2012-08-31 21:45:26 UTC
Erik,

How hard would it be to properly handle array deletions and things inside of each upload? We basically need to make sure that the reservedIndex members are sequential, at the very least by the end of the selection process....or maybe at the beginning of startUploads?
Comment 6 Mark Holmquist 2012-08-31 21:58:08 UTC
Answer: Not too hard. Try this: https://gerrit.wikimedia.org/r/22273
Comment 7 Derk-Jan Hartman 2012-09-04 22:24:53 UTC
There is still an unprotect access to uploads[0] on line 761 of mw.UploadWizard.js
Comment 8 Ryan Kaldari 2012-09-06 05:01:01 UTC
22273 doesn't fix the problem, unfortunately. The problem is caused by the call to showNext() that was added to mw.UploadWizardUpload.js here:
https://gerrit.wikimedia.org/r/#/c/20814/13/resources/mw.UploadWizardUpload.js

Unfortunately, removing that call breaks (in certain cases) the new warning overriding feature that was added.

The basic problem is that the ordering of the images (and how they are added into the uploads array) has been complicated by the fact that we now allow users to override uploads that fail due to warnings. Because of this feature, we don't actually know what images are going to get used until the user clicks the Continue button and advances to the deeds step. There are a few different ways this can be fixed.

1. Quick and dirty - disable warning overriding until we can rejigger the relevant pieces.

2. A bit complicated - Rejigger things now. Mainly we need to un-overload the showNext() function. This function is being used for several different purposes now, and some of them conflict with each other. We should move the appending functionality and the copyMetadata functionality out of showNext() and instead trigger them when the user advances to the deeds step (see moveToStep). Secondly, we shouldn't hard-code copyMetadata to uploads[0]. There are several cases where this will fail. Instead, once the user had advanced to the deeds step (or the details step), show the copyMetadataCtrlDiv for whichever upload is currently first in the list (this might require a little retooling of the copyMetadata functions as well).

3. There are a few hackish (but lower risk) ways we could patch things up without significantly changing how showNext works. I haven't totally thought these through, but one way would be to build a custom success handler for the case where a warning is overridden and remove the showNext call from the regular setSuccess method. This wouldn't solve all the issues, but it would probably solve most of the common scenarios.
Comment 9 Erik Moeller 2012-09-06 06:27:27 UTC
If we can safely remove the warning override option (we only trigger it for duplicate-archive cases, right?), I'd suggest we do that for now so we can fix the regression and then implement a proper fix later.
Comment 10 Ryan Kaldari 2012-09-06 07:52:46 UTC
Yes, it's only for duplicate-archives cases currently. If we remove the override option, we can also safely remove the showNext call from the setSuccess method, which should eliminate the inconsistent ordering.
Comment 11 Mark Holmquist 2012-09-06 16:03:34 UTC
I'm always for fixing things now, but if we only have time for the quick option, I'd prefer number 1 to number 3 I think. Thanks, kaldari.
Comment 12 Ryan Kaldari 2012-09-06 17:39:40 UTC
Temporary fix checked in for today's deployment:
https://gerrit.wikimedia.org/r/#/c/22921/
Comment 13 Derk-Jan Hartman 2012-10-28 19:54:17 UTC
Biggest part of 2: https://gerrit.wikimedia.org/r/#/c/30443/
Comment 14 Nischay Nahata 2013-05-28 14:30:37 UTC
https://gerrit.wikimedia.org/r/#/c/65318/ attempts to re-enable the option to upload previously deleted files

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links