Page MenuHomePhabricator

Unable to unprotect pages protected with earlier versions of MediaWiki
Closed, ResolvedPublic

Description

On bug 13113 it was reported that some of the templates in question (which needed to get fixed to work with the new parser) were protected but the administrator couldn't unprotect them. An example is [[fr:Modèle:Réf Livre]]; as you may natoice, deleting and restoring the page was used as a remedy for the problem.

A few days ago, similar issue was practiced with [[fa:رده:نیاز به بازبینی انسان]], which again was worked out by deleting and restoring the page. Both of the example pages were protected in mid- to late-2006. What I think is, like many other backward compatability issues, the software code should be touched so it would use any opportunity (like when a page is edited) to update the old-style protection information.

I don't know what exactly is wrong, but I think historical memory of older developers can help us figure this out.


Version: 1.12.x
Severity: normal

Details

Reference
bz13132

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:07 PM
bzimport set Reference to bz13132.
bzimport added a subscriber: Unknown Object (MLST).

mysql> SELECT DISTINCT page_restrictions FROM page;
+---------------------------------------+

page_restrictions

+---------------------------------------+

sysop
move=sysop
edit=sysop:move=sysop
edit=autoconfirmed:move=autoconfirmed
edit=autoconfirmed:move=sysop
move=sysop:edit=sysop
move=:edit=

+---------------------------------------+
8 rows in set (15.80 sec)
(on ruwiki)

I think migration script should be made for it.

With r31232 I created a maintenance script to deal with it (which can be used for Wikimedia wikis after a fast review). I tried "SELECT page_restrictions, COUNT(page_restrictions) FROM page WHERE page_restrictions<>'' GROUP BY page_restrictions;" on some of the older Wikimedia wikis and learned that there are only a few hundred to a few thousand of such protected pages on them. For example:

English Wikipedia has about 9000 such pages,
German Wikipedia has about 8000 of them,
French Wikipedia has about 3500 of them,
Persian Wikipedia ahs about 700 of them,
Russian Wikipedia has about 400 of them.

My estimate is running the maintenance script won't take much time and effort. All the same, if agreement is that letting the wiki software handle it by itself is a better choice, we can use the sae technique inside Article.php

You know, there are already two migration scripts. Make sure you check maintenance/ before making new script to avoid that.

(In reply to comment #3)

You know, there are already two migration scripts. Make sure you check
maintenance/ before making new script to avoid that.

So, why did nobody run it?

rotemliss wrote:

Apart from the script (which should be run, though the restrictions field in Special:Export breaks with the page_restrictions table), I fixed the bug itself in r31243.

Obviously, if one unprotects a page deleting all the page_restrictions rows about this page when there are no such rows (as the information is stored in page.page_restrictions field), no rows are changed in the table, the protection doesn't occur (according to the code) and the page table is not updated. Thus, I removed the duplicate check about changes in the page_restrictions table, since this is already checked using the $changed variable. Briefly testing, it seems to work.

Removed extra script in r31275.

Reverted r31243, r31244 in r31276. They cause a regression, spewing bogus entries into the page history when no change is made to protection settings on protection form submission.

rotemliss wrote:

(In reply to comment #7)

Reverted r31243, r31244 in r31276. They cause a regression, spewing bogus
entries into the page history when no change is made to protection settings on
protection form submission.

It shouldn't happen, since if the $changed variable is false (and it is false if no change is made in the form), no changes are made, the change is not logged and no entries are added to the page history. I also tested it again in a private wiki (with the fix applied), and no entries were added to the page history. How do you reproduce this regression?

(In reply to comment #8)

It shouldn't happen, since if the $changed variable is false (and it is false
if no change is made in the form), no changes are made, the change is not
logged and no entries are added to the page history. I also tested it again in
a private wiki (with the fix applied), and no entries were added to the page
history. How do you reproduce this regression?

Some of the lines removed in r31243 (which are now reverted by Brion) were added by me, a while ago, to fix for another bug. When I first noticed r31243, I thought removing those lines would not be appropriate (with a similar reasoning Brion mentioned when reverting it). However, I couldn't prove myself, because none of the tests I ran on my test wiki failed. I'm just as curious as Rotem is here, about what I'm missing which makes me unable to reproduce the regression.

rotemliss wrote:

(In reply to comment #9)

(In reply to comment #8)

It shouldn't happen, since if the $changed variable is false (and it is false
if no change is made in the form), no changes are made, the change is not
logged and no entries are added to the page history. I also tested it again in
a private wiki (with the fix applied), and no entries were added to the page
history. How do you reproduce this regression?

Some of the lines removed in r31243 (which are now reverted by Brion) were
added by me, a while ago, to fix for another bug. When I first noticed r31243,
I thought removing those lines would not be appropriate (with a similar
reasoning Brion mentioned when reverting it). However, I couldn't prove myself,
because none of the tests I ran on my test wiki failed. I'm just as curious as
Rotem is here, about what I'm missing which makes me unable to reproduce the
regression.

I see you added them in r30158. This actually fixed another bug which was caused by thsese lines - the $dbw->affectedRows() call only referred to the last query done, but didn't change the intended meanings of the lines (which is to avoid logging the change when no changes are done).

I have now found the lines were added in r30020, because of bug 12716. I could reproduce this bug (reverting the fix) by creating a new page and unprotecting it. I fixed this bug in r31322. The problem was that Title::mRestrictionsExpiry was NULL rather than "infinity" when neither old-fashioned restrictions nor restrictions from the page_restrictions table were found (i.e. the page was not protected). It seems to be more logical to set Title::mRestrictionsExpiry to "infinity" when the page is not protected - the "restriction" never expires. Thus, it is now done initially and unconditionally. An alternative solution would be to change line 1728 in Article.php:

		$changed = $changed || ($this->mTitle->mRestrictionsExpiry != $expiry);

to:

		$changed = $changed || ($this->mTitle->mRestrictionsExpiry != $expiry && !($this->mTitle->mRestrictionsExpiry == null && $expiry == Block::infinity()));

but I consider this extra condition ugly and unnecessary.