Page MenuHomePhabricator

Upload Wizard should check for protection status of titles
Closed, ResolvedPublic

Description

Rightly or wrongly, the Wikimedia Commons community uses protection on pages like "File:Desert.jpg" to enforce meaningful titles.

Upload Wizard should ideally notify the user of this as part of the other checks (title blacklist, title existence, etc.) while the user is typing and surface the protection reason through the UI. At minimum it should enable the user to correct the issue in the final step. Right now it fails in the final step with "[api-error-protectedpage]".


Version: unspecified
Severity: normal

Details

Reference
bz37107

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:21 AM
bzimport added a project: UploadWizard.
bzimport set Reference to bz37107.

I think this conflicts with another solution--UW won't always send the actual filename to the server, because it might cause unrecoverable errors, and suppress errors we care about. In order to solve _this_ problem, we would need to know about the filename on the server side, so unless we want to either send two API requests (which is technically possible, but not ideal) or modify the API to make it accept two filenames, one ideal and one actual (which is nice, but might break other things), then we are stuck here for a bit.

We're already doing dynamic title existence checks in the "Describe" step, why don't we just add it there?

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

OK, after revisiting (finally) I've discovered that we already get an error back on the last step, so it should be relatively simple to handle it. Standby.

Testing in production, unfortunately issues with protected titles persist.

Steps to reproduce:

  1. Pick a protected title (use protection log to identify one with sysop-level protection enabled) and ensure you're logged in without sufficient privileges to edit protected pages. I picked File:Rene.jpg as a filename.
  1. Ensure the file you're uploading wouldn't trigger other issues (e.g. previously deleted).
  1. Try to advance through UploadWizard to upload.

Expected behavior: It should prompt me to rename the file, ideally in the third ("Describe") step.

Actual behavior: I'm getting the pop-up of doom, "please wait, still checking the title for uniqueness" (in Chrome 21.0.1180.81) ad infinitum and can't advance past the "Describe" step.

patch was merged, can this be closed?

No, as I wrote in the comment right above yours, this issue is not resolved.

sorry, skimmed too fast. Thanks

Created attachment 11008
worksforme

It works for me on the master branch. Perhaps the change was not yet deployed when Erik tested it ?

Attached:

Screen_Shot_2012-08-26_at_02.05.46.png (199×913 px, 24 KB)

Or there may have been caching issues? I know ResourceLoader can be naughty about caching, especially when testing things very quickly.

With latest master, I get proper errors....a few versions behind Erik, but Chromium nonetheless. Some more testing is necessary, I think. Erik, can you do more digging on this please?

I'm testing in production, with both Chrome and Firefox. I'm still getting the issue pointed out in comment 6, with the test file being a random image renamed to "Rene.jpg" (which is a currently protected title). I've validated in Chrome's dev tools that the JS file loaded is indeed the version that includes https://gerrit.wikimedia.org/r/#/c/21155/

Pop-up of doom should be fixed in the trunk version (along with a lot of other error handling). The current behavior in the case of protected pages is that the UploadWizard will error at the very last step and then let the user change the file name. Ideally, we should have it make the check when it is doing the other filename checks in the Describe step as suggested by Erik.

ideally, we need to add something to the API to 'testrun' the upload, because we now need separate api checks for everything that can possibly go wrong.

Instead we should have a mode where it just goes trough all the checks but doesn't 'commit' the file and page. We then have all title, protection, blocklogs, illegal filenames and what not handled in the SAME way as the final upload will report. Instead of using 4 separate checks on in our mw.DestinationChecker (which then runs the danger of getting out of sync with 'core'

DJ, I think the thing we need is to add in protection-checking to the TitleBlacklist / uniqueness API call(s) that we already perform. It should be relatively simple, but will take a core change.

Until then, I'd say, the while-uploading check is our best bet.

Change abandoned, due to lack of time by author to guide this trough the process.

Before I start working on this one, let me clear a confusion. A page that is protected will already be a duplicate name (already used) so that error would pop-up, right?

No. The issue is that you can protect pages that do not exist - or files that don't exist - and so we need to check for both errors.

I am trying to fix this by adopting the change by DJ in https://gerrit.wikimedia.org/r/56371
I am yet to test this though.

The change is merged and this is fixed. However I have one doubt:

If a page named File:Hello.jpg is protected the error "This title corresponds to a protected page on this wiki. Please choose a different one." pops up on entering the image title "Hello" as expected. However, it would be better to say

"The title corresponds to a protected page File:Hello.jpg on this wiki. Please choose a different one."

The ".jpg" part can lead to some confusion. If anybody else thinks this is a possible problem lets open a new bug for it.

Gilles raised the priority of this task from Medium to Unbreak Now!.Dec 4 2014, 10:23 AM
Gilles added a project: Multimedia.
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to Medium.Dec 4 2014, 11:22 AM