From 753cfc15532a39b835a77ce4bd9bdc86581180e0 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 29 Jun 2005 06:16:03 +0000 Subject: [PATCH] * Various code cleanup and HTML escaping fixlets on page history, contribs etc * fix page history with table prefix * fix paging on history * switch 'earliest' and 'latest' link order to fix 'prev' and 'next' * use null where appropriate * switch some messages to plaintext or wikitext --- RELEASE-NOTES | 1 + includes/PageHistory.php | 85 ++++++++++++++----------- includes/SpecialContributions.php | 49 +++++++------- includes/SpecialMovepage.php | 20 +++--- includes/SpecialRecentchanges.php | 6 +- includes/SpecialRecentchangeslinked.php | 4 +- includes/WebRequest.php | 14 ++++ languages/Language.php | 4 +- 8 files changed, 103 insertions(+), 80 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 0a39f0f203..57d5fa238f 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -418,6 +418,7 @@ Various bugfixes, small features, and a few experimental things: * (bug 2584) Fix output of subcategory list * (bug 2597) Don't crash when undeleting an image description page * (bug 2564) Don't show "editingold" warning for recent revision +* Various code cleanup and HTML escaping fixlets === Caveats === diff --git a/includes/PageHistory.php b/includes/PageHistory.php index 06ee5fca8c..6791e71380 100644 --- a/includes/PageHistory.php +++ b/includes/PageHistory.php @@ -77,7 +77,7 @@ class PageHistory { */ $id = $this->mTitle->getArticleID(); if( $id == 0 ) { - $wgOut->addHTML( wfMsg( 'nohistory' ) ); + $wgOut->addWikiText( 'nohistory' ); wfProfileOut( $fname ); return; } @@ -338,35 +338,36 @@ class PageHistory { } } - function getFirstOffset($id) { - $db =& wfGetDB(DB_SLAVE); - $sql = "SELECT MAX(rev_timestamp) AS lowest FROM revision WHERE rev_page = $id"; - $res = $db->query($sql, "getFirstOffset"); - $obj = $db->fetchObject($res); - return $obj->lowest; + function getLatestOffset($id) { + return $this->getExtremeOffset( $id, 'max' ); + } + + function getEarliestOffset($id) { + return $this->getExtremeOffset( $id, 'min' ); } - function getLastOffset($id) { + function getExtremeOffset( $id, $func ) { $db =& wfGetDB(DB_SLAVE); - $sql = "SELECT MIN(rev_timestamp) AS lowest FROM revision WHERE rev_page = $id"; - $res = $db->query($sql, "getLastOffset"); - $obj = $db->fetchObject($res); - return $obj->lowest; + return $db->selectField( 'revision', + "$func(rev_timestamp)", + array( 'rev_page' => $id ), + 'PageHistory::getExtremeOffset' ); } - function getLastOffsetForPaging($id, $step = 50) { + function getLastOffsetForPaging( $id, $step = 50 ) { $db =& wfGetDB(DB_SLAVE); - $sql = "SELECT rev_timestamp FROM revision WHERE rev_page = $id " . + $revision = $db->tableName( 'revision' ); + $sql = "SELECT rev_timestamp FROM $revision WHERE rev_page = $id " . "ORDER BY rev_timestamp ASC LIMIT $step"; - $res = $db->query($sql, "getLastOffsetForPaging"); - $n = $db->numRows($res); + $res = $db->query( $sql, "PageHistory::getLastOffsetForPaging" ); + $n = $db->numRows( $res ); - if ($n == 0) - return NULL; - - while ($n--) - $obj = $db->fetchObject($res); - return $obj->rev_timestamp; + $last = null; + while( $obj = $db->fetchObject( $res ) ) { + $last = $obj->rev_timestamp; + } + $db->freeResult( $res ); + return $last; } function getDirection() { @@ -451,8 +452,9 @@ wfdebug("limits=$limits offset=$offsets got=".count($result)."\n"); $revisions = array_slice($revisions, 0, $limit); - $abslowts = $this->getLastOffset($this->mTitle->getArticleID()); - $abshights = $this->getFirstOffset($this->mTitle->getArticleID()); + $pageid = $this->mTitle->getArticleID(); + $latestTimestamp = $this->getLatestOffset( $pageid ); + $earliestTimestamp = $this->getEarliestOffset( $pageid ); /* * When we're displaying previous revisions, we need to reverse @@ -468,9 +470,9 @@ wfdebug("limits=$limits offset=$offsets got=".count($result)."\n"); $lowts = $hights = 0; - if (count($revisions)) { - $lowts = $revisions[0]->rev_timestamp; - $hights = $revisions[count($revisions) - 1]->rev_timestamp; + if( count( $revisions ) ) { + $latestShown = $revisions[0]->rev_timestamp; + $earliestShown = $revisions[count($revisions) - 1]->rev_timestamp; } $firsturl = $wgTitle->escapeLocalURL("action=history&limit={$limit}&go=first"); @@ -478,29 +480,36 @@ wfdebug("limits=$limits offset=$offsets got=".count($result)."\n"); $firsttext = wfMsg("histfirst"); $lasttext = wfMsg("histlast"); - $prevurl = $wgTitle->escapeLocalURL("action=history&dir=prev&offset={$lowts}&limit={$limit}"); - $nexturl = $wgTitle->escapeLocalURL("action=history&offset={$hights}&limit={$limit}"); + $prevurl = $wgTitle->escapeLocalURL("action=history&dir=prev&offset={$latestShown}&limit={$limit}"); + $nexturl = $wgTitle->escapeLocalURL("action=history&offset={$earliestShown}&limit={$limit}"); $urls = array(); foreach (array(20, 50, 100, 250, 500) as $num) { $urls[] = "escapeLocalURL( "action=history&offset={$offset}&limit={$num}")."\">".$wgLang->formatNum($num).""; } -wfDebug("lowts=$lowts abslowts=$abslowts\n"); + $bits = implode($urls, ' | '); - if ($lowts < $abslowts) { - $prevtext = "".wfMsg("prevn", $limit).""; + + wfDebug("latestShown=$latestShown latestTimestamp=$latestTimestamp\n"); + if( $latestShown < $latestTimestamp ) { + $prevtext = "".wfMsgHtml("prevn", $limit).""; $lasttext = "$lasttext"; - } else $prevtext = wfMsg("prevn", $limit); + } else { + $prevtext = wfMsgHtml("prevn", $limit); + } - if ($hights < $abshights) { - $nexttext = "".wfMsg("nextn", $limit).""; + wfDebug("earliestShown=$earliestShown earliestTimestamp=$earliestTimestamp\n"); + if( $earliestShown > $earliestTimestamp ) { + $nexttext = "".wfMsgHtml("nextn", $limit).""; $firsttext = "$firsttext"; - } else $nexttext = wfMsg("nextn", $limit); + } else { + $nexttext = wfMsgHtml("nextn", $limit); + } - $firstlast = "($firsttext | $lasttext)"; + $firstlast = "($lasttext | $firsttext)"; - return "$firstlast " . wfMsg("viewprevnext", $prevtext, $nexttext, $bits); + return "$firstlast " . wfMsgHtml("viewprevnext", $prevtext, $nexttext, $bits); } } diff --git a/includes/SpecialContributions.php b/includes/SpecialContributions.php index fc90a43bf1..9272ede239 100644 --- a/includes/SpecialContributions.php +++ b/includes/SpecialContributions.php @@ -17,8 +17,7 @@ function wfSpecialContributions( $par = null ) { // GET values $target = isset($par) ? $par : $wgRequest->getVal( 'target' ); - $namespace = $wgRequest->getVal( 'namespace', '' ); - $namespace = $namespace === '' ? NULL : $namespace; + $namespace = $wgRequest->getIntOrNull( 'namespace' ); $invert = $wgRequest->getBool( 'invert' ); $hideminor = ($wgRequest->getBool( 'hideminor' ) ? true : false); @@ -63,20 +62,19 @@ function wfSpecialContributions( $par = null ) { $id = 0; } - $wgOut->setSubtitle( wfMsg( 'contribsub', $ul ) ); + $wgOut->setSubtitle( wfMsgHtml( 'contribsub', $ul ) ); - if ( $hideminor ) { - $minorQuery = "AND rev_minor_edit=0"; - $mlink = $sk->makeKnownLink( $wgContLang->specialPage( "Contributions" ), - WfMsg( "show" ), "target=" . htmlspecialchars( $nt->getPrefixedURL() ) . - "&offset={$offset}&limit={$limit}&hideminor=0&namespace={$namespace}" ); - } else { - $minorQuery = ""; - $mlink = $sk->makeKnownLink( $wgContLang->specialPage( "Contributions" ), - WfMsg( 'hide' ), 'target=' . htmlspecialchars( $nt->getPrefixedURL() ) . - "&offset={$offset}&limit={$limit}&hideminor=1&namespace={$namespace}" ); - } + $contribsPage = Title::makeTitle( NS_SPECIAL, 'Contributions' ); + $mlink = $sk->makeKnownLinkObj( $contribsPage, + $hideminor ? wfMsgHtml( 'show' ) : wfMsgHtml( 'hide' ), + wfArrayToCGI( array( + 'target' => $nt->getPrefixedDbKey(), + 'offset' => $offset, + 'limit' => $limit, + 'hideminor' => $hideminor ? 0 : 1, + 'namespace' => $namespace ) ) ); + $minorQuery = $hideminor ? "AND rev_minor_edit=0" : ""; if( !is_null($namespace) ) { $minorQuery .= ' AND page_namespace ' . ($invert ? '!' : '') . "= {$namespace}"; } @@ -112,12 +110,12 @@ function wfSpecialContributions( $par = null ) { "hideminor=$hideminor&namespace=$namespace&invert=$invert&target=" . wfUrlEncode( $target ), ($numRows) <= $limit); - $shm = wfMsg( "showhideminor", $mlink ); + $shm = wfMsgHtml( "showhideminor", $mlink ); $wgOut->addHTML( "
{$sl} ($shm)

\n"); if ( 0 == $numRows ) { - $wgOut->addHTML( "\n

" . wfMsg( "nocontribs" ) . "

\n" ); + $wgOut->addWikiText( wfMsg( "nocontribs" ) ); return; } @@ -225,28 +223,29 @@ function ucNamespaceForm ( $target, $hideminor, $namespace, $invert ) { global $wgContLang, $wgScript; $namespaceselect = "'; - $out = "
"; - $out .= ''; - $out .= ''; - $out .= ''; + $action = htmlspecialchars( $wgScript ); + $out = "
"; + $out .= ''; + $out .= ''; + $out .= ''; $out .= "
- + $namespaceselect - + - +
"; $out .= '
'; return $out; diff --git a/includes/SpecialMovepage.php b/includes/SpecialMovepage.php index 9fbf2b1ade..447eaf82e1 100644 --- a/includes/SpecialMovepage.php +++ b/includes/SpecialMovepage.php @@ -95,12 +95,12 @@ class MovePageForm { if ( $err == 'articleexists' && $wgUser->isAllowed( 'delete' ) ) { $wgOut->addWikiText( wfMsg( 'delete_and_move_text', $encNewTitle ) ); - $movepagebtn = wfMsg( 'delete_and_move' ); + $movepagebtn = wfMsgHtml( 'delete_and_move' ); $submitVar = 'wpDeleteAndMove'; $err = ''; } else { $wgOut->addWikiText( wfMsg( 'movepagetext' ) ); - $movepagebtn = wfMsg( 'movepagebtn' ); + $movepagebtn = wfMsgHtml( 'movepagebtn' ); $submitVar = 'wpMove'; } @@ -108,10 +108,10 @@ class MovePageForm { $wgOut->addWikiText( wfMsg( 'movepagetalktext' ) ); } - $movearticle = wfMsg( 'movearticle' ); - $newtitle = wfMsg( 'newtitle' ); - $movetalk = wfMsg( 'movetalk' ); - $movereason = wfMsg( 'movereason' ); + $movearticle = wfMsgHtml( 'movearticle' ); + $newtitle = wfMsgHtml( 'newtitle' ); + $movetalk = wfMsgHtml( 'movetalk' ); + $movereason = wfMsgHtml( 'movereason' ); $titleObj = Title::makeTitle( NS_SPECIAL, 'Movepage' ); $action = $titleObj->escapeLocalURL( 'action=submit' ); @@ -119,7 +119,7 @@ class MovePageForm { if ( $err != '' ) { $wgOut->setSubtitle( wfMsg( 'formerror' ) ); - $wgOut->addHTML( '

' . wfMsg($err) . "

\n" ); + $wgOut->addWikiText( '

' . wfMsg($err) . "

\n" ); } $moveTalkChecked = $this->moveTalk ? ' checked="checked"' : ''; @@ -259,13 +259,13 @@ class MovePageForm { $wgRawHtml = $marchingantofdoom; if ( $talkmoved == 1 ) { - $wgOut->addHTML( "\n

" . wfMsg( 'talkpagemoved' ) . "

\n" ); + $wgOut->addWikiText( wfMsg( 'talkpagemoved' ) ); } elseif( 'articleexists' == $talkmoved ) { - $wgOut->addHTML( "\n

" . wfMsg( 'talkexists' ) . "

\n" ); + $wgOut->addWikiText( wfMsg( 'talkexists' ) ); } else { $ot = Title::newFromURL( $oldtitle ); if ( ! $ot->isTalkPage() ) { - $wgOut->addHTML( "\n

" . wfMsg( 'talkpagenotmoved', wfMsg( $talkmoved ) ) . "

\n" ); + $wgOut->addWikiText( wfMsg( 'talkpagenotmoved', wfMsg( $talkmoved ) ) ); } } } diff --git a/includes/SpecialRecentchanges.php b/includes/SpecialRecentchanges.php index 4024a005cd..8ad4899d7e 100644 --- a/includes/SpecialRecentchanges.php +++ b/includes/SpecialRecentchanges.php @@ -34,7 +34,7 @@ function wfSpecialRecentchanges( $par, $specialPage ) { /* bool */ 'hideliu' => false, /* bool */ 'hidepatrolled' => false, /* text */ 'from' => '', - /* text */ 'namespace' => '', + /* text */ 'namespace' => null, /* bool */ 'invert' => false, ); @@ -64,7 +64,7 @@ function wfSpecialRecentchanges( $par, $specialPage ) { } else { - $namespace = $wgRequest->getVal( 'namespace', $defaults['namespace'] ); + $namespace = $wgRequest->getIntOrNull( 'namespace' ); $invert = $wgRequest->getBool( 'invert', $defaults['invert'] ); $hidebots = $wgRequest->getBool( 'hidebots', $defaults['hidebots'] ); $hideliu = $wgRequest->getBool( 'hideliu', $defaults['hideliu'] ); @@ -131,7 +131,7 @@ function wfSpecialRecentchanges( $par, $specialPage ) { $hidem .= $hidebots ? ' AND rc_bot=0' : ''; $hidem .= $hideliu ? ' AND rc_user=0' : ''; $hidem .= $hidepatrolled ? ' AND rc_patrolled=0' : ''; - $hidem .= $namespace === '' ? '' :' AND rc_namespace' . ($invert ? '!=' : '=') . $namespace; + $hidem .= is_null( $namespace ) ? '' : ' AND rc_namespace' . ($invert ? '!=' : '=') . $namespace; // This is the big thing! diff --git a/includes/SpecialRecentchangeslinked.php b/includes/SpecialRecentchangeslinked.php index 2717d63c00..7a8b2ad0e1 100644 --- a/includes/SpecialRecentchangeslinked.php +++ b/includes/SpecialRecentchangeslinked.php @@ -68,7 +68,7 @@ function wfSpecialRecentchangeslinked( $par = NULL ) { if( $nt->getNamespace() == NS_CATEGORY ) { $catkey = $dbr->addQuotes( $nt->getDBKey() ); $sql = - "SELECT page_id,page_namespace,page_title,rev_user,rev_comment, + "SELECT page_id,page_namespace,page_title,rev_id,rev_user,rev_comment, rev_user_text,rev_timestamp,rev_minor_edit, page_is_new FROM $categorylinks, $revision, $page @@ -85,7 +85,7 @@ ORDER BY rev_timestamp DESC } else { $sql = "SELECT page_id,page_namespace,page_title, - rev_user,rev_comment,rev_user_text,rev_timestamp,rev_minor_edit, + rev_user,rev_comment,rev_user_text,rev_id,rev_timestamp,rev_minor_edit, page_is_new FROM $pagelinks, $revision, $page WHERE rev_timestamp > '{$cutoff}' diff --git a/includes/WebRequest.php b/includes/WebRequest.php index f55a4b7adf..31195ebe55 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -175,6 +175,20 @@ class WebRequest { return IntVal( $this->getVal( $name, $default ) ); } + /** + * Fetch an integer value from the input or return null if empty. + * Guaranteed to return an integer or null; non-numeric input will + * typically return null. + * @param string $name + * @return int + */ + function getIntOrNull( $name ) { + $val = $this->getVal( $name ); + return is_numeric( $val ) + ? IntVal( $val ) + : null; + } + /** * Fetch a boolean value from the input or return $default if not set. * Guaranteed to return true or false, with normal PHP semantics for diff --git a/languages/Language.php b/languages/Language.php index 045f26f867..4cef69fb4c 100644 --- a/languages/Language.php +++ b/languages/Language.php @@ -1567,9 +1567,9 @@ to move a page.", 'articleexists' => 'A page of that name already exists, or the name you have chosen is not valid. Please choose another name.', -'talkexists' => 'The page itself was moved successfully, but the +'talkexists' => "'''The page itself was moved successfully, but the talk page could not be moved because one already exists at the new -title. Please merge them manually.', +title. Please merge them manually.'''", 'movedto' => 'moved to', 'movetalk' => 'Move "talk" page as well, if applicable.', 'talkpagemoved' => 'The corresponding talk page was also moved.', -- 2.20.1