Page MenuHomePhabricator

Implement rev_deleted (deletion of single article revisions)
Closed, ResolvedPublic

Description

Author: tietew-mediazilla

Description:
I created a patch to implement revision deletion (rev_deleted) via
Special:Revisiondelete.

Japanese Wikipedia needs this feature very very much.

  • Article.php
    • If the revision is deleted, returns 'deletedrevisiontext' instead of its content.
  • DifferenceEngine.php
    • If one of or both revisions are deleted, hide differences.
  • LogPage.php
    • Added new action 'delete/revdelete' and 'delete/revrestore'
  • PageHistory.php
    • Added a link to Special:Revisiondelete at the bottom of the page.
  • SpecialExport.php
    • Put <deleted/> element if the revision is deleted.
  • SpecialImport.php
    • Handle <deleted/> element.
  • SpecialPage.php
    • Added Special:Revisiondelete.
  • SpecialRevisiondelete.php
    • New. Implements revision deletion and restoration.
  • Language.php
    • Added 6 new messages used by this patch.

tested at http://test.wikipedia.jp/history/Revision_deletion


Version: unspecified
Severity: enhancement

Details

Reference
bz3576

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 8:52 PM
bzimport set Reference to bz3576.

tietew-mediazilla wrote:

Implementing rev_deleted feature

attachment revisiondelete.patch ignored as obsolete

tietew-mediazilla wrote:

Implementing rev_deleted feature

regenerated patch; fixed some mistakes, etc.

attachment revisiondelete.patch ignored as obsolete

Seems to be working without fatal errors, but needs some polishing.

For example:
Notice: Undefined offset: 3324 in
/services/www/mediawiki15c/includes/SpecialRevisiondelete.php on line 165

Notice: Undefined variable: header in
/services/www/mediawiki15c/includes/SpecialRevisiondelete.php on line 99

Notice: Undefined property: rev_timestamp in
/services/www/mediawiki15c/includes/SpecialUndelete.php on line 186

avarab wrote:

(In reply to comment #2)

Created an attachment (id=934) [edit]
Implementing rev_deleted feature

regenerated patch; fixed some mistakes, etc.

Some problems with it:

  • $wgOut->addHTML( $header . $s ); : omit $header, the variable is undefined
  • When I delete revisions is spews warnings, the code in question is:
    • if( $revisions[ (int)$row->rev_id ] ) { (line 165)
  • I think displaying ''This revision is deleted.'' in the textbox is bad form,

there's no way to tell if the revision is really deleted or if somebody just
wrote that and saved a revision without looking at the history, I think that
"This revision is deleted." should be displayed on rendering but now when you go
into the source to avoid the ambiguity.

  • Use wfMsgHtml not htmlspecialchars( wfMsg( ..
  • You changed the export format, then you have to updated the export format,

make a new docs/export-0.4.xsd based on docs/export-0.3.xsd and define <deleted/>

avarab wrote:

(In reply to comment #4)

(In reply to comment #2)
"This revision is deleted." should be displayed on rendering but now when you go

  • but not..

tietew-mediazilla wrote:

Implementing rev_deleted feature

Patch renewed.

(In reply to comment #4)

  • $wgOut->addHTML( $header . $s ); : omit $header, the variable is undefined

fixed.

  • When I delete revisions is spews warnings, the code in question is:
    • if( $revisions[ (int)$row->rev_id ] ) { (line 165)

may be fixed.

  • I think displaying ''This revision is deleted.'' in the textbox is bad

form,

there's no way to tell if the revision is really deleted or if somebody just
wrote that and saved a revision without looking at the history, I think that
"This revision is deleted." should be displayed on rendering but now when you

go

into the source to avoid the ambiguity.

action=edit just shows "This revision isdeleted." and omit form.
action=raw returns empty.

  1. oops, title=Foo&oldid=xxx&direction=prev links to
  2. title=Foo&action=edit&oldid=xxx instead of oldid=(prev of xxx)

--> http://test.wikipedia.jp/edit/Revision_deletion_DEMO_page?oldid=2609

  • You changed the export format, then you have to updated the export format,

make a new docs/export-0.4.xsd based on docs/export-0.3.xsd and define

<deleted/>
docs/export-0.4.xsd added.

(In reply to comment #3)

Notice: Undefined property: rev_timestamp in
/services/www/mediawiki15c/includes/SpecialUndelete.php on line 186

This patch does not include SpecialUndelete.php.

attachment revisiondelete.patch ignored as obsolete

avarab wrote:

(In reply to comment #6)

Created an attachment (id=995) [edit]
Implementing rev_deleted feature

  • When I delete revisions is spews warnings, the code in question is:
    • if( $revisions[ (int)$row->rev_id ] ) { (line 165)

may be fixed.

It's not fixed, please run PHP with "error_reporting = E_ALL" in your php.ini,
in your solution you made a typo which caused revision deletion to not work at
all, it should be:

166                         $rev_id = (int)$row->rev_id;
167                         if( @$revisions[ $rev_id ] ) {

not

166                         $rev_id = (int)$row->rev_id;
167                         if( $revisions[ $rev ] ) {

Please exclude whitespace fixes and fixes to the code not directly related to
this patch, examples include adding whitespace and changing htmlspecialchars(
wfMsg() ) to wfMsgHtml(), please send seperate cleanup patches instead.

I like how you handled the "This revision is deleted." problem, nice work.

tietew-mediazilla wrote:

Implementing rev_deleted feature

Patch renewed, cleaned up.
No warnings found with error_reporting = E_ALL

attachment revisiondelete.patch ignored as obsolete

avarab wrote:

(In reply to comment #8)

Created an attachment (id=999) [edit]
Implementing rev_deleted feature

Patch renewed, cleaned up.
No warnings found with error_reporting = E_ALL

The patch fails to apply to CVS HEAD

avarab wrote:

Removing need-review and patch keywords, reviewed and the patch fails to apply.

tietew-mediazilla wrote:

Implementing rev_deleted feature

Patch renewed for current HEAD.

Conflicts are fixed in following files (no feature changes):

  • DifferenceEngine.php
  • EditPage.php
  • Export.php

Modified:

  • Language.php, SpecialRevisiondelete.php
    • add new message 'revisiondeletebutton' for submit button

attachment revisiondelete_1205.patch ignored as obsolete

tietew-mediazilla wrote:

Adding need-review and patch keywords; patch renewed for current HEAD.

gangleri wrote:

Made some tests at:
http://test.wikipedia.jp/Test:Bugzilla/03576

To be able to test yourself please ask Tietew for sysop status at:
irc://irc.freenode.net/wikipedia-ja or
http://test.wikipedia.jp/Test:Administrators#requests_for_additional_user_rights

best regards reinhardt [[user:gangleri]]

avarab wrote:

Assigning the bug to myself, working on cleaning up the patch to make it sutable
for inclusion.

gangleri wrote:

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

avarab wrote:

(In reply to comment #14)

Assigning the bug to myself, working on cleaning up the patch to make it sutable
for inclusion.

Stopped working on this some time ago, the patch attached here needs to be
improved in a lot of ways

  • Having delete/undelete in the page history view would be much simpler, easier

to implement and would use the current paging code, one isn't likely to need to
mass-delete individual revisions

  • It should act correctly if the latest revision or all revisions are deleted

rowan.collins wrote:

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

robchur wrote:

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

robchur wrote:

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

tietew-mediazilla wrote:

see also:
http://mail.wikipedia.org/pipermail/wikitech-l/2006-February/034170.html
[Wikitech-l] Updating rev_deleted as a bitfield

kelly.lynn.martin wrote:

This functionality, or something similar to it, is desparately needed on the
English Wikipedia.

robchur wrote:

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

lar wrote:

Agree with Kelly, there's a good reason this bug/feature fix would come in
REALLY handy right now. Cleaning out a few edits from the history of a 5000 edit
article is very difficult to do reliably and efficiently by all reports. So any
enhancements would be greatly appreciated. ++Lar

emddudley wrote:

Desperately needed, ASAP. 5000? Bah, try the en:AN or en:AN/I. Over 20000 and 30000!

I second the above. This should probably be one of the top priorities.

snoutwood wrote:

I concur entirely with Voice of All, Mdd4696, and Kelly Martin. I can't think of
anything else I'd consider a higher priority (although Bug 550 regarding blocks
for anons only comes close).

ayg wrote:

Please do not make posts that do not have to do with the patch's development.
They send people unnecessary e-mails and clog up the thread so that devs have a
harder time finding relevant info. If you would like to express your support,
just vote for the bug.

This functionality exists for users with oversight anyway, what's the issue?
(Note that replying to that question is relevant, since it clarifies the desired
featureset. ;) )

snoutwood wrote:

(In response to comment #27)

Please do not make posts that do not have to do with the patch's development.

Understood, sorry, I won't do it again.

This functionality exists for users with oversight anyway, what's the issue?

Seventeen users with the permission isn't enough to deal with the number of
cases in which this comes up. It would be an extremely handy feature for
postings of personal information and copyvios, and there's no reason that this
should be restricted to so few users as those assigned oversight and not all
admins, who can due this now anyway, albeit with significant drawbacks.

The importance of this feature is that there are often times when deletion of a
single edit, or only a few edits, is required. Currently, the only way to do
that is to delete the entire page and restore all of the other edits, which has
the following problems: 1) while the page is deleted it is inaccessible to other
users, 2) it is very difficult on large pages (I nearly crashed my computer
trying to delete [[WP:AN/I]] to remove some edits containing personal
information), 3) one cannot use diffs once the page is deleted to check that the
edit they're trying to delete is in fact the correct one, which can emerge as a
problem on long pages, and 4) one has to click on a MESS of boxes to undelete
large pages, depending (sometimes one can shift-select of use javascript
workarounds, but it's still a pain).

ayg wrote:

In that case, what you want is for oversight permission to be granted to all
bureaucrats, perhaps, or all sysops, or whatever. This can be done with current
software, it's a policy issue and should be discussed on VPP or wheverever.

If you want the edits to be recoverable, though, that I can understand and agree
with as an eventual goal. I've changed the summary to clarify what the current
situation is.

(Removed needs-review, patch: patch is close to a year out of date and will have
to be rewritten. Changed priority from High back to Normal: this is noncritical
with the oversight feature available in the code.)

snoutwood wrote:

Yes, the idea is *definitely* for the edits to be recoverable (I think that all
sysops having oversight is a majorly bad idea).

wiki_tomos wrote:

This feature was developed by Tietew in the context of problems experienced by
Japanese Wikipedia. I believe that ja community would still prefer edits to be
recoverable - just like deleted pages, deleted revisions should be recoverable
and be subject to inspections when in doubt. (Besides, for some reason, oversight
permission is granted on en wikipedia, not elsewhere, according to the page on
meta. http://meta.wikimedia.org/wiki/Hiding_revisions ).

But the more important reason why this feature is needed is that it will hide
the content of the revision, while keeping the history visible to the public.
Oversight permission as I understand does not do that. And keeping history
information (who contributed to the page and when) is a simple way to comply
with GFDL.

Imagine that you find an infringing material in a past revision of an article,
and that infringing part is inhereted by subsequent revisions down to
the very current one. In the meantime, many valuable contributions are made to
the article. What you can do with this feature is to edit out all the infringing
parts so that the current one becomes clean, and suppress the text of all the
revisions containing the infringement, but keeping the revision history intact,
so that we have on the record who contributed to the current revision of the
article. In contrast, if without this feature, their names will be deleted along
with the revisions they contributed, and reflecting their contributions to the
current revision without proper attribution to them in the revision history is a
violation of GFDL (4I).

If my explanation is not clear, please let me know (in person or here), I am
more than happy to explain it to defend the value of this feature.

In a nutshell, this feature enables us to do things oversight permission cannot
do. It is a simple way to preserve some past contributions that are mixed with
past infringement. Some Japanese Wikipedians like me have been discussing about
this feature from time to time, waiting for the implementation for a long time.

robchur wrote:

Resetting summary. This bug is still quite valid. The HideRevisions extension is
not part of the core code, and this problem needs to be addressed.

To those trumpeting "oversight", please be aware of this. To those attempting to
defend, don't panic. "Oversight" is a short-term solution at present.

Please could people refrain from commenting or altering fields on this bug
unless you are a developer or acting under the instructions of one.

ayg wrote:

(In reply to comment #32)

Resetting summary. This bug is still quite valid. The HideRevisions extension is
not part of the core code, and this problem needs to be addressed.

To those trumpeting "oversight", please be aware of this. To those attempting to
defend, don't panic. "Oversight" is a short-term solution at present.

Please could people refrain from commenting or altering fields on this bug
unless you are a developer or acting under the instructions of one.

Sorry, will do.

tietew-mediazilla wrote:

Increasing priority.
ja.wikipedia needs this feature strongly.

ayg wrote:

Priority is determined by developers, not the users. It's a way of them telling
you what they plan to do, not you telling them what you want them to do. If you
believe this is an important feature, vote for it and/or submit a patch. This
may or may not speed things along, because MediaWiki is a volunteer project: it
will get done when a dev feels like doing it. Setting the priority field
certainly won't help anything (it's basically not used here anyway).

Rob Church, by the way, *is* a developer.

treauth wrote:

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

PageHistory.php - better handling of deleted usernames

attachment hist.patch ignored as obsolete

DiffEng - better handling of diffs with deleted text

attachment diffeng_rd.patch ignored as obsolete

Comment on attachment 3000
PageHistory.php - better handling of deleted usernames

Visual improvements done in r19763 and r19765

Completion patch

First draft

Schema:
-Adds ar_deleted, log_deleted, rc_deleted, rc_logid
-Also adds log_type and log_action to recentchanges for later use (bug 5546)

Code changes:
-Changelists - permission checks added
-SpecialLog - permission checks added
-Linker - some log functions and public/private function arguments added
-DiffEng - cleaner behavoir
-Article - rev_deleted -> ar_delete on archival and some minor stuff
-Revisiondelete - support for log and archive delete, no more duplicate logs,
and proper message/param use.
-Undelete - permission checks
-Defaultsettings, specialpages & specialspecialpages - new log restriction
system. Used for oversight log.
-Other stuff scattered here and there...

attachment revisiondelete.patch ignored as obsolete

Completion patch

Cleaned some things up. Made logs more user friendly.

attachment revisiondelete.patch ignored as obsolete

Completion patch

Clean up event deletion logging

attachment revisiondelete.patch ignored as obsolete

Completion patch

Clean up some things

attachment revisiondelete.patch ignored as obsolete

Fix some warning errors

attachment revisiondelete.patch ignored as obsolete

Add user block hiding

-Add uption to blockip to hide name from block log, ipblocklist, and listusers.
The block entry goes to the oversight log and users with 'hiderevision' can see
the ipblocklist items.
-Allow for quick, clean, total page suppression. Oversights can do the opposite
by checking an option at special:undelete.
-More cleanup: recentchanges styling, revisiondelete checkboxes, ect...

attachment revisiondelete.patch ignored as obsolete

Fix sql cond grouping

Also note:
-Instead of rc_log_action I have rc_actiontext to separate log action text from
log comments to better carry over log hiding.
-The rc_log_type is now used and some slight watchlist sql query changes now
allow for watchlist to show log events (bug 5546).

attachment revisiondelete.patch ignored as obsolete

Add fa_deleted for archived images and some UI cleanup

attachment revisiondelete.patch ignored as obsolete

Enforce for RSS feed data

attachment revisiondelete.patch ignored as obsolete

Resolve merge conflict with listusers

I may just make this a branch soon...

attachment revisiondelete.patch ignored as obsolete

robchur wrote:

(In reply to comment #49)

I may just make this a branch soon...

Please do so; the sooner the better. BugZilla isn't good for version control. ;)

Comment on attachment 3272
Resolve merge conflict with listusers

(In reply to comment #50)

(In reply to comment #49)

I may just make this a branch soon...

Please do so; the sooner the better. BugZilla isn't good for version control. ;)

Indeed, I have taken over the old rev_delete branch here:
http://svn.wikimedia.org/viewvc/mediawiki/branches/phase3_rev_deleted/?sortby=d
ate

Wiki.Melancholie wrote:

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

rotemliss wrote:

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

suisui wrote:

why $wgGroupPermissions['sysop']['deleterevision'] = true;
still keeps comment out on DefaultSettings.php?
something problems yet?

rotemliss wrote:

It was reverted in r20525.

(In reply to comment #55)

why $wgGroupPermissions['sysop']['deleterevision'] = true;
still keeps comment out on DefaultSettings.php?
something problems yet?

Note really, I should probably uncomment that when this goes in MW 1.11.

Ryanpostlethwaite wrote:

Given the recent problems en.wiki has been having, we would really like to have this ASAP. We're aware that the extension is a little buggy at the minute, but is there any way to hurry it up?

(In reply to comment #58)

Given the recent problems en.wiki has been having, we would really like to have
this ASAP. We're aware that the extension is a little buggy at the minute, but
is there any way to hurry it up?

It is not complete at all now. I'd have to commit a lot of code. All of this has been held up in code review for months, as no one will have the time to look it over.

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

Why can't sysops use this on Wikimedia projects? I get a permission error when I go on [[Special:Revisiondelete]]. Isn't it ready?

MediaWiki wrote:

I have a feeling that it's limited to bureaucrats. I haven't had a chance to see any documentation yet, but IIRC the default (commented-out) settings in DefaultSettings.php of MW 1.11.1 had the right assigned to bureaucrats.

(In reply to comment #62)

Why can't sysops use this on Wikimedia projects? I get a permission error when
I go on [[Special:Revisiondelete]]. Isn't it ready?

Aphaia wrote:

Either bureaucrats seems not to be allowed. I tried it on jawikinews where I am a b'crat but said "You are not allowed to execute the action you have requested."

MediaWiki wrote:

It could also be that the new core code simply hasn't been synchronized with the WMF servers yet. I keep hoping someone more knowledgeable will notice this thread and answer your questions, Aphaia and Pietrodn...

(In reply to comment #65)

It could also be that the new core code simply hasn't been synchronized with
the WMF servers yet. I keep hoping someone more knowledgeable will notice this
thread and answer your questions, Aphaia and Pietrodn...

Both. It is not synced, a schema change is pending. Also, it will be tested and reviewed more before being turned on.

gmaxwell wrote:

Looks like this got turned on shortly on EnWP because there are now revisions with non-zero rev-deleted and no clear way to get them out of this state: http://en.wikipedia.org/w/index.php?title=User:FT2/pdr&action=history

It was the intention that these revisions ultimately end up oversighted, but as it stands now oversight users have no way of viewing them. :)

nykevin.norris wrote:

I can't find RevisionDelete at [[Special:Version]], but the above links show it's working... am I blind?

skizzerz wrote:

you wouldn't find it at Special:Version because it is core functionality, not an extension. Only extensions are listed on that page