dupe

• This bug has been marked as a duplicate of bug 27470 ***

This is not a dupe. Upload protection for non existing files is a core functionality.

This is a problem with r79655.

When title does not exist, the 'upload' restriction is not added any more.

Bryan.TongMinh wrote:

r82853

I’m sorry, but the functionality still does not seem to work. See http://commons.wikimedia.org/wiki/Special:Log?page=File:Wiki.png

The first thing I noticed in the code is that in verifyTitlePermissions, “createpage” is checked, while ProtectionForm works with “create”. (Which might not be the problem, I didn’t analyze the code fully.)

Also note the protection form is broken: for nonexisting files, the “upload” protection seems to be offered, but not saved (in ProtectionForm.php:save(), in the “!$this->mTitle->exists()” case, only $this->mRestrictions['create'] is used). (Not sure if this should be split to another bug.)

Bryan.TongMinh wrote:

Well, the code changes were not yet synced to Wikimedia yet, so that explains that it still does not work.

Created attachment 8825
This still seems to be broken

(In reply to comment #6)

Well, the code changes were not yet synced to Wikimedia yet, so that explains
that it still does not work.

Oh, dear. You mean having such an important bug fix not being deployed for _half a year_ would be an acceptable explanation? Never mind, all bug fixes I know about _have_ been deployed and http://svn.wikimedia.org/viewvc/mediawiki/branches/wmf/1.17wmf1/RELEASE-NOTES?view=markup lists bug 27700 as fixed.

And testing the behavior on the current trunk yields still the same problem.

I don’t want to dig into the overcomplicated code, but patching Title::checkActionPermissions so that the “$action == 'create'” test tests also for “$action == 'createpage'” seems to fix the problem (not saying this is a correct fix, just pointing to the problem). See the attached patch.

Attached:

Bryan.TongMinh wrote:

Oh right, I forgot that this that revision was indeed synced to Wikimedia. Will have a look at it later.

(In reply to comment #8)

Will have a look at it later.

Any idea _when_ “later”? You know… there is this image which is kept uploaded again and again on Commons…

http://commons.wikimedia.org/wiki/Special:Log?page=File:Wiki.png

Bryan.TongMinh wrote:

(In reply to comment #9)

(In reply to comment #8)

Will have a look at it later.

Any idea _when_ “later”? You know… there is this image which is kept uploaded
again and again on Commons…

http://commons.wikimedia.org/wiki/Special:Log?page=File:Wiki.png

When I have time. Unlikely to be in the nearby future. For quick resolution it's probably better to find somebody else to poke at this.

I'll see if I can get someone to look at this before 1.18 (no promises). I don't feel strongly that this is high priority or that it should be a 1.18 blocker, but I'm willing to assume it is. Moremegil, you seem to feel this is very high priority; could you describe the problems this is causing (not just the theoretical, but maybe somehow quantify the admin load)?

(In reply to comment #11)

I'll see if I can get someone to look at this before 1.18 (no promises). I
don't feel strongly that this is high priority or that it should be a 1.18
blocker, but I'm willing to assume it is. Moremegil, you seem to feel this is
very high priority; could you describe the problems this is causing (not just
the theoretical, but maybe somehow quantify the admin load)?

Well, this seems to me rather a _visible regression_, a single user-visible functionality does not work at all (you go to ?action=protect and whatever you set there has no effect whatsoever).

I don’t think this is “very high” _priority_, I would consider it to be blocker for a _release_ (but I am in no position to make such decisions).

I am unable to quantify the admin load, I am aware only of the “upload-protect/uploaded anyway/delete/upload-protect/uploaded anyway” cycle for [[commons:File:Wiki.png]] (as I participated there); by looking at the protection log, I could find [[commons:File:Adam.jpg]] or [[commons:File:Flickr.jpg]], but those are randomly found examples, not real data.

If you couldn’t find someone to solve the bug “properly”, at least consider applying the attached patch, which is probably just a dirty hack, but at least makes it _work_.

taking this off deployment blocker status, making tarball blocker, assigning while Aaron reviews it

Page doesn't seems to make sense. Callers should only check 'create' and the proper permission ('createpage' or 'createtalk') should be handled in checkQuickPermissions().

According to the docs, the checkQuickPermissions function should "Check action permissions not already checked in checkQuickPermissions".

This bug may have appeared in r65898.

Should be fixed in r96601.