Page MenuHomePhabricator

UploadWizard doesn't highlight errors in "more info"
Closed, ResolvedPublic

Description

Author: neilk

Description:
On the UploadWizardDetails page, we are now validating the data in the Latitude, Longitude and Altitude section. However, although the fields are properly noted with red errors, these are hidden behind a "more info" drop-down section.

So if the submission is prevented while that section is closed, it's not obvious to the user how to fix it.

Preferred behavior: pop open "more info" if it has errors.

Or add another red warning to indicate the user should pop it open.


Version: unspecified
Severity: major

Details

Reference
bz32408

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 12:04 AM
bzimport added a project: UploadWizard.
bzimport set Reference to bz32408.

Arguably it would be also desirable to not insert obviously incorrect information.

For example, sometimes the altitude seems to be filled by a camera with non-numeric fields. Is there really any value in inserting this data into the form?

Numbers should also be trimmed of whitespace and obvious conversions made, if that's an issue (dots vs. commas).

See:
http://commons.wikimedia.org/wiki/Commons:Upload_Wizard_feedback#Trim_Altitude_and_specify_lat.2Flong_format_to_be_inserted.21

saibotrash wrote:

A user reported that he tried to upload [[:File:Museum in Frashër, Albania - Original.jpg]] in UW but UW complained about about errors. He couldn't see that it was because of the height field since it is hidden. He asked me to report the confusing error. So, here we are.

Please unhide all boxes in case of errors therein! This bug known since at least two months - but not fixed... NO new features without capacity to maintain them - thanks.

saibotrash wrote:

/edit (stupid bugzilla).. +":commons"

neilk wrote:

ok, so there are two annoying bugs here.

  1. EXIF coordinate data is stored in a "rational" format that looks like a set of fractions. Deep in the innards of our EXIF parser, latitude and longitude are converted from rational to decimal, but altitude is not. Why? I asked and the answer is "because it's simpler this way". Whatever.

Solution to this may be to fix the EXIF lib.

  1. It doesn't pop open the 'more info' box when there are errors.

Obvious issue, been noted for a long time, only is becoming a bigger issue because of the altitude thing.

neilk wrote:

*** Bug 33433 has been marked as a duplicate of this bug. ***

  • Bug 39640 has been marked as a duplicate of this bug. ***

Derk-Jan, I would suggest that it would be relatively easy to change the code of the error-showing method to look for the closest parent with a certain selector and make sure it's visible. I'd be happy to take a look at this on Monday, when I have more time.

Thanks for your work on this stuff this weekend, it all looks really great.

PDF of UW page in this state

Looks like I just ran into this in my first attempt to use the upload wizard.

Tried uploading three files. The UW complains about three errors, but doesn't show me what it thinks was wrong. I don't see anything amiss.

(Note: I copied the info from the first file to the other two via the "Copy information to all uploads below ..." function.)

I'll now have to re-do these three uploads through the normal upload page.

This renders the upload wizard completely useless to me. (If you think that was too harsh, please consider that I'm an experienced uploader, not some newbie, and a developer. Yet I'm completely baffled and have no idea what I should do. Granted, I may be a newbie in UW usage... but I thought UW should make uploading easier and be intuitive for newbies?)

BTW, I also have found no way to upload the files as "own work", but released under their real license {{PD-ineligible}}. Which means I would have had to re-edit all three file description pages after the upload. That's not user-friendly.

Attached:

Just noticed that the three titles in the form had no file extension. Presumably the UW figures that out by itself? In any case, adding ".png" to the three titles didn't change anything.

I can reproduce this in Chrome 22. It seems to be a recently introduced bug if you chose to set the license individually for each file in the first step. Honestly, that feature has always been prone to bugs and I'd prefer to remove it completely, although it'd be good to have data on how widely it's used first.

In the meantime, we should fix the current bug. What seems to be happening here is that the validators get run against the "source" and "author" fields of the "not my own work" licensing path, even if that path isn't selected. These get flagged as warnings, but because the "not my own work" path isn't expanded the user has no idea what's going on.

(In reply to comment #13)

Attempt to fix in
https://gerrit.wikimedia.org/r/#/c/36342/

Showing hidden parts that contain errors is certainly a good idea. However, in my case the problem was not with the additional info, and anyway, if you go that route, I think it should be done in a general way (find all error divs, check that they're all visible, and if not, make them visible by ensuring that the whole parent hierarchy is shown). A special case just for the additional info part won't fix this completely.

In my case, I had, in search for a way to enter the custom license "{{PD-ineligible}}", clicked once on "This is not my own work". When I did that, input fields for source, author, etc. were added to the form. I didn't enter anything, but chose again "This is my own work". At that point, the "not own work" fields in the form got hidden, but they remained in the form's DOM. They didn't have sensible values, though, and thus final validation failed.

If I understood comment 12 correctly, the problem in this case is that the whole form is validated finally via the jQuery validation plugin, which just calls validators on all fields in the form. It doesn't know about the logical hierarchy or about fields being hidden but just goes through all form inputs and calls a validation method.

Now, I see two ways to properly fix this in mw.UploadWizardDeed.js:

  1. the individual deeds' validators must check whether the deed is selected at all and just return true if not, or
  1. the deed selector must disable the de-selected deeds' form input (presumably, the jQuery validation plugin is smart enough to skip disabled inputs) and explicitly enable a selected deed's inputs.

Similar problems may exist in other places where fields are added to the form, but shall be ignored because some other input's setting actually means "ignore this field".

(In reply to comment #14)

In my case, I had, in search for a way to enter the custom license
"{{PD-ineligible}}", clicked once on "This is not my own work". When I did
that, input fields for source, author, etc. were added to the form. I didn't
enter anything, but chose again "This is my own work". At that point, the "not
own work" fields in the form got hidden, but they remained in the form's DOM.
They didn't have sensible values, though, and thus final validation failed.

I was not able to reproduce this. I clicked once on "This is not my own work" entered a too short source and clicked on next, the field was highlighted with red and showed the error clearly. I then chose "This is my own work" and clicked on "next" and the form worked without any error.

In step "Upload", choose two files. Choose "Provide individual licenses" in the "Release rights" step. Then, in the describe step, click "Own work" for each of them. Don't even touch the "not own work". Provide dummy descriptions. Click "Next". I get the error then. To see the errors, click on "Not own work": UW wants to have the source fields to be set.

So it appears that it's even simpler, but the basic problem is the same: the form contains inputs that don't need validation because some other settings effectively say "these fields are not used" (namely the radio boxes for the deed selection). Only the inputs of the selected deeds must be checked, while inputs of de-selected deeds must not be validated.

(In reply to comment #16)
Thanks for the detailed reply, I now see the problem. Sounds like these are two separate bugs

  1. The error fields are in a hidden collapsible
  2. Unnecessary fields are validated

But my patch seems to work for 1 only

Both patches are merged now. Nischay's patch expands collapsed sections containing form validation errors; my patch prevents deselected deeds from being validated at all. Together, this should resolve this bug.

Gilles raised the priority of this task from High to Unbreak Now!.Dec 4 2014, 10:21 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to High.Dec 4 2014, 11:21 AM