* 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

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 0a39f0f..57d5fa2 100644 (file)
--- 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 06ee5fc..6791e71 100644 (file)
--- 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[] = "<a href=\"".$wgTitle->escapeLocalURL(
                                "action=history&offset={$offset}&limit={$num}")."\">".$wgLang->formatNum($num)."</a>";
                }
-wfDebug("lowts=$lowts abslowts=$abslowts\n");
+               
                $bits = implode($urls, ' | ');
-               if ($lowts < $abslowts) {
-                       $prevtext = "<a href=\"$prevurl\">".wfMsg("prevn", $limit)."</a>";
+               
+               wfDebug("latestShown=$latestShown latestTimestamp=$latestTimestamp\n");
+               if( $latestShown < $latestTimestamp ) {
+                       $prevtext = "<a href=\"$prevurl\">".wfMsgHtml("prevn", $limit)."</a>";
                        $lasttext = "<a href=\"$lasturl\">$lasttext</a>";
-               } else  $prevtext = wfMsg("prevn", $limit);
+               } else {
+                       $prevtext = wfMsgHtml("prevn", $limit);
+               }
 
-               if ($hights < $abshights) {
-                       $nexttext = "<a href=\"$nexturl\">".wfMsg("nextn", $limit)."</a>";
+               wfDebug("earliestShown=$earliestShown earliestTimestamp=$earliestTimestamp\n");
+               if( $earliestShown > $earliestTimestamp ) {
+                       $nexttext = "<a href=\"$nexturl\">".wfMsgHtml("nextn", $limit)."</a>";
                        $firsttext = "<a href=\"$firsturl\">$firsttext</a>";
-               } 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 fc90a43..9272ede 100644 (file)
--- 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( "<br />{$sl} ($shm)</p>\n");
 
 
        if ( 0 == $numRows ) {
-               $wgOut->addHTML( "\n<p>" . wfMsg( "nocontribs" ) . "</p>\n" );
+               $wgOut->addWikiText( wfMsg( "nocontribs" ) );
                return;
        }
 
@@ -225,28 +223,29 @@ function ucNamespaceForm ( $target, $hideminor, $namespace, $invert ) {
        global $wgContLang, $wgScript;
 
        $namespaceselect = "<select name='namespace' id='nsselectbox'>";
-       $namespaceselect .= '<option value="" '.(is_null($namespace) ? ' selected="selected"' : '').'>'.wfMsg( 'contributionsall' ).'</option>';
+       $namespaceselect .= '<option value="" '.(is_null($namespace) ? ' selected="selected"' : '').'>'.wfMsgHtml( 'contributionsall' ).'</option>';
        $arr = $wgContLang->getFormattedNamespaces();
        foreach( $arr as $ns => $name ) {
                if( $ns < NS_MAIN )
                        continue;
-               $n = $ns === NS_MAIN ? wfMsg ( 'blanknamespace' ) : $name;
+               $n = $ns === NS_MAIN ? wfMsgHtml( 'blanknamespace' ) : htmlspecialchars( $name );
                $sel = $namespace == $ns ? ' selected="selected"' : '';
                $namespaceselect .= "<option value='$ns'$sel>$n</option>";
        }
        $namespaceselect .= '</select>';
 
-       $out = "<div class='namespaceselector'><form method='get' action='{$wgScript}'>";
-       $out .= '<input type="hidden" name="title" value="'.$wgContLang->specialpage( 'Contributions' ).'" />';
-       $out .= '<input type="hidden" name="target" value="'.htmlspecialchars( $target ).'" />';
-       $out .= '<input type="hidden" name="hideminor" value="'.$hideminor.'" />';      
+       $action = htmlspecialchars( $wgScript );
+       $out = "<div class='namespaceselector'><form method='get' action=\"$action\">";
+       $out .= '<input type="hidden" name="title" value="' . htmlspecialchars( $wgContLang->specialpage( 'Contributions' ) ) . '" />';
+       $out .= '<input type="hidden" name="target" value="' . htmlspecialchars( $target ) . '" />';
+       $out .= '<input type="hidden" name="hideminor" value="' . ( $hideminor ? 1 : 0 ) .'" />';       
        $out .= "
 <div id='nsselect' class='contributions'>
-       <label for='nsselectbox'>" . wfMsg('namespace') . "</label>
+       <label for='nsselectbox'>" . wfMsgHtml('namespace') . "</label>
        $namespaceselect
-       <input type='submit' value='" . wfMsg( 'allpagessubmit' ) . "' />
+       <input type='submit' value=\"" . wfMsgHtml( 'allpagessubmit' ) . "\" />
        <input type='checkbox' name='invert' value='1' id='nsinvert'" . ( $invert ? ' checked="checked"' : '' ) . " />
-       <label for='nsinvert'>" . wfMsg('invert') . "</label>
+       <label for='nsinvert'>" . wfMsgHtml('invert') . "</label>
 </div>";
        $out .= '</form></div>';
        return $out;

diff --git a/includes/SpecialMovepage.php b/includes/SpecialMovepage.php
index 9fbf2b1..447eaf8 100644 (file)
--- 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( '<p class="error">' . wfMsg($err) . "</p>\n" );
+                       $wgOut->addWikiText( '<p class="error">' . wfMsg($err) . "</p>\n" );
                }
 
                $moveTalkChecked = $this->moveTalk ? ' checked="checked"' : '';
@@ -259,13 +259,13 @@ class MovePageForm {
                $wgRawHtml = $marchingantofdoom;
 
                if ( $talkmoved == 1 ) {
-                       $wgOut->addHTML( "\n<p>" . wfMsg( 'talkpagemoved' ) . "</p>\n" );
+                       $wgOut->addWikiText( wfMsg( 'talkpagemoved' ) );
                } elseif( 'articleexists' == $talkmoved ) {
-                       $wgOut->addHTML( "\n<p><strong>" . wfMsg( 'talkexists' ) . "</strong></p>\n" );
+                       $wgOut->addWikiText( wfMsg( 'talkexists' ) );
                } else {
                        $ot = Title::newFromURL( $oldtitle );
                        if ( ! $ot->isTalkPage() ) {
-                               $wgOut->addHTML( "\n<p>" . wfMsg( 'talkpagenotmoved', wfMsg( $talkmoved ) ) . "</p>\n" );
+                               $wgOut->addWikiText( wfMsg( 'talkpagenotmoved', wfMsg( $talkmoved ) ) );
                        }
                }
        }

diff --git a/includes/SpecialRecentchanges.php b/includes/SpecialRecentchanges.php
index 4024a00..8ad4899 100644 (file)
--- 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 2717d63..7a8b2ad 100644 (file)
--- 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 f55a4b7..31195eb 100644 (file)
--- 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 045f26f..4cef69f 100644 (file)
--- 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.',