Extension talk:PureWikiDeletion

Have you considered empty rather than a special string like #DELETED? Ashley Y 08:45, 10 March 2010 (UTC)
 * It's a possibility. Some editors have pointed out that #DELETED makes it clear that the intent is to delete rather than vandalize, but a vandal could just as easily replace all content with #DELETED. It saves keystrokes to use empty too. On the other hand, if we use #DELETED, then blanking is unnecessary; we can just add a #DELETED tag to the beginning of a page and leave the rest of the content, so that people don't have to dig through the history to find what was there. Then again, that possibility also exists with #REDIRECT, but in practice almost everyone replaces all the existing page content with #REDIRECT. Tisane 23:19, 10 March 2010 (UTC)

Blanking should be logged?
The current spec says: The blanking of a page already appears on watchlists and recent changes, so I think this is redundant and unnecessary. However, it would probably be useful to be able to get a list of all blanked pages and filter and sort it by user and time of blanking, as we do with the deletion logs. So, I will create a special page with that functionality, and I figure out a way to filter the blanked pages out of AllPages. (Right now it shows up in AllPages as redlinks.) Tisane 09:24, 11 March 2010 (UTC)
 * The replacing of all content on a page with "#DELETED," or the removal of "#DELETED" from a page should be logged, and listed on watchlists and recent changes like the other logs (the move log, the protection log, etc.)

Are all these features necessary?

 * Blank pages will not show up in searches using Special:Search, random pages using Special:Randompage, or the list of all pages using Special:Allpages.
 * The URL prefix to a blank page should be listed in robots.txt, and the page will emit "noarchive" and "noindex" and "nofollow" tags, to prevent caching in search engines.

I can see why people wouldn't want it to show up in RandomPage or AllPages (although I haven't figured out yet how to implement that). But given that these pages are blanked, won't the external search engines stop directing people there anyway? Also, it still shows up in the internal search as "page title matches," but as a redlink. Maybe it's not so bad for it to continue showing up there. If it's a search term people are looking for a lot, maybe the article should remain there, or at least be a redirect? Maybe it should show up in search accompanied by the edit summary explaining why it was blanked. Tisane 12:42, 13 March 2010 (UTC)

htmlentities
OK, is that good enough to clear up the XSS issues? Tisane 17:56, 13 March 2010 (UTC)

Code review
Since there is some interest in getting this used on Wikipedia, I'll provide some initial code review, though it will still need review from a staff developer before it can be enabled.

setup code
'version' => '0.0.0', If you aren't going to give a useful number, you might as well just leave it blank.

$wgGroupPermissions['*']['PWD']   = false; It isn't necessary to explicitly set it to false, as that's the default. Also, this right doesn't seem to do anything.

function PWDSaveHook
global $wgOut; This isn't used anywhere

require( "extensions/PWD/PWDConfig.php" ); All configuration should be done in LocalSettings

$con = mysql_connect($yourHost,$yourUsername,$yourPassword); You should use the wiki database rather than creating your own, see Manual:Hooks/LoadExtensionSchemaUpdates for how to set up the database using update.php. MediaWiki also has its own classes for dealing with the database. See Manual:Database access

$sql=sprintf("INSERT INTO blanked_pages (blanked_page, blanking_user, blanking_timestamp, After you switch to using MediaWiki's database classes, you should use the wrapper functions for queries, as these automatically handle quoting and escaping

mysql_real_escape_string($article->getTitle)); Article::getTitle returns a Title object, not a string. As written, it should use $article->getTitle->getPrefixedDBkey, but it should really be storing the title and namespace index separately or just the pageid.

function PWDLink
global $wgRRAnonOnly, $wgUser,$wgOut,$wgDBserver,$wgDBuser,$wgDBpassword,$wgTitle, $wgServer, $wgScriptPath;

AFICT, $wgRRAnonOnly doesn't actually exist and none of these are used.

$PWDActivated=true; if ($PWDActivated==false){ $PWDActivated will never be false because you set it to true in the line directly above. Also, you should use either !$PWDActivated or $PWDActivated===false, depending on whether you need type checking as well

if( $target->isKnown ) { return true; } else { return true; } Just "return true" is shorter.

if ( in_array( 'known', $options ) ) { The existence check may not have been done at this point. You should probably use the LinkEnd hook.

function PWDEditHook
$date = date('M d, Y H:i:s', mktime($hour, $minute, $second, $month, $day, $year)); You should pass the timestamp directly to wfTimestamp

$user_url=$wgServer.$wgScriptPath.$URLStructForUserPage.$userPage; Use $wgUser->getSkin->link to make a link. Same for all the other links

$output=" A former version of this page was blanked by ".$user_HTML This should set a message in the i18n file and call it with something like wfMsg

parser function functions
These should call and check $parser->incrementExpensiveFunctionCount since they need to do a database query for each usage.

return htmlentities($param2); You shouldn't escape the output from the parser functions, as it still needs to go through the parser

PWD_body.php
p_ID int NOT NULL AUTO_INCREMENT, This should use the same prefix "blanked_" as the other fields

$sql="CREATE INDEX userind on blanked_pages (blanking_user)"; As far as I can see, "blanked_page" is the only thing used in a WHERE condition, so none of the other fields need an index.

PWDConfig.php
As noted, all configuration should be done in LocalSettings, and it shouldn't use its own database.

date_default_timezone_set ( 'America/Denver' ); This shouldn't be set in an extension unless there's a really good reason

Code review discussion
You may want to look over pages like Manual:Coding conventions. Mr.Z-man 05:02, 14 March 2010 (UTC)
 * Thanks for the code review; I will get to work on these items immediately. Tisane 07:19, 15 March 2010 (UTC)

Known bug
There is a known bug involving the improper use of $wgUser that causes the user and talk pages on the edit screen to be incorrectly linked, but it will be fixed in version 1.0.0. Tisane 02:53, 19 March 2010 (UTC)

Draft code posted
I posted some draft code. Have at it, Kim. If you start doing a major overhaul, you may wish to use the Template:Inuse. And feel free to add yourself to the introductory comments as a co-author. Tisane 03:50, 19 March 2010 (UTC)

More Code Review
Just a point about your SQL schema. Fields within a table should use a consistent prefix; currently some of the fields have "blanked" and some use "blanking". The prefix should also be short; I'd suggest something like "blank". Also, timestamps should be varbinary(14), and ids should be UNSIGNED INT:

Happy ‑ melon 13:14, 19 March 2010 (UTC)