r79083 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r79082‎ \| r79083 \| r79084 >
Date:	19:07, 27 December 2010
Author:	happy-melon
Status:	ok (Comments)
Tags:
Comment:	Refactoring in RecentChanges/Watchlist: * Use new jQuery/RL collapsible implementation, remove legacy JS. * Refactor ChangesList::flag() and ChangesList::recentChangesFlags(), the latter to now take an array rather than a growing list of out-of-order parameters. * Resolve the FIXME requested in r44421 (just passed its second birthday :D) by replacing extract() calls with direct references.
Modified paths:	/trunk/phase3/includes/ChangesList.php (modified) (history) /trunk/phase3/resources/Resources.php (modified) (history) /trunk/phase3/resources/mediawiki.special/mediawiki.special.changeslist.css (added) (history) /trunk/phase3/skins/common/enhancedchanges.js (deleted) (history) /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/enhancedchanges.js
—	—	@@ -1,40 +0,0 @@
2		-/*
3		~~- JavaScript file for enhanced recentchanges~~
4		~~- */~~
5		-
6		-/*
7		~~- * Add the CSS to hide parts that should be collapsed~~
8		- *
9		~~- * We do this with JS so everything will be expanded by default~~
10		~~- * if JS is disabled~~
11		~~- */~~
12		~~-appendCSS('.mw-changeslist-hidden {'+~~
13		~~- ' display:none;'+~~
14		~~- '}'+~~
15		~~- 'div.mw-changeslist-expanded {'+~~
16		~~- ' display:block;'+~~
17		~~- '}'+~~
18		~~- 'span.mw-changeslist-expanded {'+~~
19		~~- ' display:inline !important;'+~~
20		~~- ' visibility:visible !important;'+~~
21		~~- '}'~~
22		~~-);~~
23		-
24		-/*
25		~~- * Switch an RC line between hidden/shown~~
26		~~- * @param int idNumber : the id number of the RC group~~
27		~~-*/~~
28		~~-window.toggleVisibility = function(idNumber) {~~
29		~~- var openarrow = document.getElementById("mw-rc-openarrow-"+idNumber);~~
30		~~- var closearrow = document.getElementById("mw-rc-closearrow-"+idNumber);~~
31		~~- var subentries = document.getElementById("mw-rc-subentries-"+idNumber);~~
32		~~- if (openarrow.className == 'mw-changeslist-expanded') {~~
33		~~- openarrow.className = 'mw-changeslist-hidden';~~
34		~~- closearrow.className = 'mw-changeslist-expanded';~~
35		~~- subentries.className = 'mw-changeslist-expanded';~~
36		~~- } else {~~
37		~~- openarrow.className = 'mw-changeslist-expanded';~~
38		~~- closearrow.className = 'mw-changeslist-hidden';~~
39		~~- subentries.className = 'mw-changeslist-hidden';~~
40		~~- }~~
41		~~-};~~
Index: trunk/phase3/skins/common/shared.css
—	—	@@ -831,18 +831,6 @@
832	832	word-wrap: break-word;
833	833	}
834	834
835		~~-table.mw-enhanced-rc {~~
836		~~- background: none;~~
837		~~- border:0;~~
838		~~- border-spacing:0;~~
839		-}
840		~~-td.mw-enhanced-rc {~~
841		~~- white-space:nowrap;~~
842		~~- padding:0;~~
843		~~- vertical-align:top;~~
844		~~- font-family:monospace~~
845		-}
846		-
847	835	#mw-addcategory-prompt {
848	836	display: inline;
849	837	margin-left: 1em;
Index: trunk/phase3/includes/ChangesList.php
—	—	@@ -30,7 +30,7 @@
31	31	class ChangesList {
32	32	public $skin;
33	33	protected $watchlist = false;
34		-
	34	+
35	35	/**
36	36	* Changeslist contructor
37	37	* @param $skin Skin
—	—	@@ -81,21 +81,19 @@
82	82	}
83	83	}
84	84
85		-
86	85	/**
87	86	* Returns the appropriate flags for new page, minor change and patrolling
88		~~- * @param $new Boolean~~
89		~~- * @param $minor Boolean~~
90		~~- * @param $patrolled Boolean~~
	87	+ * @param $flags Associative array of 'flag' => Bool
91	88	* @param $nothing String to use for empty space
92		~~- * @param $bot Boolean~~
93	89	* @return String
94	90	*/
95		~~- protected function recentChangesFlags( $new, $minor, $patrolled, $nothing = ' ', $bot = false ) {~~
96		~~- $f = $new ? self::flag( 'newpage' ) : $nothing;~~
97		~~- $f .= $minor ? self::flag( 'minor' ) : $nothing;~~
98		~~- $f .= $bot ? self::flag( 'bot' ) : $nothing;~~
99		~~- $f .= $patrolled ? self::flag( 'unpatrolled' ) : $nothing;~~
	91	+ protected function recentChangesFlags( $flags, $nothing = ' ' ) {
	92	+ $f = '';
	93	+ foreach( array( 'newpage', 'minor', 'bot', 'unpatrolled' ) as $flag ){
	94	+ $f .= isset( $flags[$flag] ) && $flags[$flag]
	95	+ ? self::flag( $flag )
	96	+ : $nothing;
	97	+ }
100	98	return $f;
101	99	}
102	100
—	—	@@ -108,25 +106,31 @@
109	107	* @param $key String: 'newpage', 'unpatrolled', 'minor', or 'bot'
110	108	* @return String: Raw HTML
111	109	*/
112		~~- public static function flag( $key ) {~~
	110	+ public static function flag( $flag ) {
113	111	static $messages = null;
114	112	if ( is_null( $messages ) ) {
115		~~- foreach ( explode( ' ', 'minoreditletter boteditletter newpageletter ' .~~
116		~~- 'unpatrolledletter recentchanges-label-minor recentchanges-label-bot ' .~~
117		~~- 'recentchanges-label-newpage recentchanges-label-unpatrolled' ) as $msg ) {~~
118		~~- $messages[$msg] = wfMsgExt( $msg, 'escapenoentities' );~~
	113	+ $messages = array(
	114	+ 'newpage' => array( 'newpageletter', 'recentchanges-label-newpage' ),
	115	+ 'minoredit' => array( 'minoreditletter', 'recentchanges-label-minor' ),
	116	+ 'botedit' => array( 'boteditletter', 'recentchanges-label-bot' ),
	117	+ 'unpatrolled' => array( 'unpatrolledletter', 'recentchanges-label-unpatrolled' ),
	118	+ );
	119	+ foreach( $messages as $key => &$value ) {
	120	+ $value[0] = wfMsgExt( $value[0], 'escapenoentities' );
	121	+ $value[1] = wfMsgExt( $value[1], 'escapenoentities' );
119	122	}
120	123	}
	124	+
121	125	# Inconsistent naming, bleh
122		~~- if ( $key == 'newpage' \|\| $key == 'unpatrolled' ) {~~
123		~~- $key2 = $key;~~
124		~~- } else {~~
125		~~- $key2 = $key . 'edit';~~
126		~~- }~~
127		~~- return "<abbr class=\"$key\" title=\""~~
128		~~- . $messages["recentchanges-label-$key"] . "\">"~~
129		~~- . $messages["${key2}letter"]~~
130		~~- . '</abbr>';~~
	126	+ $map = array(
	127	+ 'newpage' => 'newpage',
	128	+ 'minor' => 'minor',
	129	+ 'bot' => 'bot',
	130	+ 'unpatrolled' => 'unpatrolled',
	131	+ );
	132	+ $flag = $map[$flag];
	133	+
	134	+ return "<abbr class='$flag' title='" . $messages[$flag][1] . "'>" . $messages[$flag][0] . '</abbr>';
131	135	}
132	136
133	137	/**
—	—	@@ -507,8 +511,15 @@
508	512	} else {
509	513	$this->insertDiffHist( $s, $rc, $unpatrolled );
510	514	# M, N, b and ! (minor, new, bot and unpatrolled)
511		~~- $s .= $this->recentChangesFlags( $rc->mAttribs['rc_new'], $rc->mAttribs['rc_minor'],~~
512		~~- $unpatrolled, '', $rc->mAttribs['rc_bot'] );~~
	515	+ $s .= $this->recentChangesFlags(
	516	+ array(
	517	+ 'newpage' => $rc->mAttribs['rc_new'],
	518	+ 'minoredit' => $rc->mAttribs['rc_minor'],
	519	+ 'unpatrolled' => $unpatrolled,
	520	+ 'botedit' => $rc->mAttribs['rc_bot']
	521	+ ),
	522	+ ''
	523	+ );
513	524	$this->insertArticleLink( $s, $rc, $unpatrolled, $watched );
514	525	}
515	526	# Edit/log timestamp
—	—	@@ -566,7 +577,7 @@
567	578	$this->rcCacheIndex = 0;
568	579	$this->lastdate = '';
569	580	$this->rclistOpen = false;
570		~~- $wgOut->addModules( 'mediawiki.legacy.enhancedchanges' );~~
	581	+ $wgOut->addModuleStyles( 'mediawiki.special.changeslist' );
571	582	return '';
572	583	}
573	584	/**
—	—	@@ -580,14 +591,10 @@
581	592	# Create a specialised object
582	593	$rc = RCCacheEntry::newFromParent( $baseRC );
583	594
584		~~- # Extract fields from DB into the function scope (rc_xxxx variables)~~
585		~~- // FIXME: Would be good to replace this extract() call with something~~
586		~~- // that explicitly initializes variables.~~
587		~~- extract( $rc->mAttribs );~~
588		~~- $curIdEq = array( 'curid' => $rc_cur_id );~~
	595	+ $curIdEq = array( 'curid' => $rc->mAttribs['rc_cur_id'] );
589	596
590	597	# If it's a new day, add the headline and flush the cache
591		~~- $date = $wgLang->date( $rc_timestamp, true );~~
	598	+ $date = $wgLang->date( $rc->mAttribs['rc_timestamp'], true );
592	599	$ret = '';
593	600	if( $date != $this->lastdate ) {
594	601	# Process current cache
—	—	@@ -599,36 +606,38 @@
600	607
601	608	# Should patrol-related stuff be shown?
602	609	if( $wgUser->useRCPatrol() ) {
603		~~- $rc->unpatrolled = !$rc_patrolled;~~
	610	+ $rc->unpatrolled = !$rc->mAttribs['rc_patrolled'];
604	611	} else {
605	612	$rc->unpatrolled = false;
606	613	}
607	614
608	615	$showdifflinks = true;
609	616	# Make article link
	617	+ $type = $rc->mAttribs['rc_type'];
	618	+ $logType = $rc->mAttribs['rc_log_type'];
610	619	// Page moves
611		~~- if( $rc_type == RC_MOVE \|\| $rc_type == RC_MOVE_OVER_REDIRECT ) {~~
612		~~- $msg = ( $rc_type == RC_MOVE ) ? "1movedto2" : "1movedto2_redir";~~
	620	+ if( $type == RC_MOVE \|\| $type == RC_MOVE_OVER_REDIRECT ) {
	621	+ $msg = ( $type == RC_MOVE ) ? "1movedto2" : "1movedto2_redir";
613	622	$clink = wfMsg( $msg, $this->skin->linkKnown( $rc->getTitle(), null,
614	623	array(), array( 'redirect' => 'no' ) ),
615	624	$this->skin->linkKnown( $rc->getMovedToTitle() ) );
616	625	// New unpatrolled pages
617		~~- } else if( $rc->unpatrolled && $rc_type == RC_NEW ) {~~
	626	+ } else if( $rc->unpatrolled && $type == RC_NEW ) {
618	627	$clink = $this->skin->linkKnown( $rc->getTitle(), null, array(),
619		~~- array( 'rcid' => $rc_id ) );~~
	628	+ array( 'rcid' => $rc->mAttribs['rc_id'] ) );
620	629	// Log entries
621		~~- } else if( $rc_type == RC_LOG ) {~~
622		~~- if( $rc_log_type ) {~~
623		~~- $logtitle = SpecialPage::getTitleFor( 'Log', $rc_log_type );~~
	630	+ } else if( $type == RC_LOG ) {
	631	+ if( $logType ) {
	632	+ $logtitle = SpecialPage::getTitleFor( 'Log', $logType );
624	633	$clink = '(' . $this->skin->linkKnown( $logtitle,
625		~~- LogPage::logName($rc_log_type) ) . ')';~~
	634	+ LogPage::logName( $logType ) ) . ')';
626	635	} else {
627	636	$clink = $this->skin->link( $rc->getTitle() );
628	637	}
629	638	$watched = false;
630	639	// Log entries (old format) and special pages
631		~~- } elseif( $rc_namespace == NS_SPECIAL ) {~~
632		~~- list( $specialName, $logtype ) = SpecialPage::resolveAliasWithSubpage( $rc_title );~~
	640	+ } elseif( $rc->mAttribs['rc_namespace'] == NS_SPECIAL ) {
	641	+ list( $specialName, $logtype ) = SpecialPage::resolveAliasWithSubpage( $rc->mAttribs['rc_title'] );
633	642	if ( $specialName == 'Log' ) {
634	643	# Log updates, etc
635	644	$logname = LogPage::logName( $logtype );
—	—	@@ -647,7 +656,7 @@
648	657	$showdifflinks = false;
649	658	}
650	659
651		~~- $time = $wgLang->time( $rc_timestamp, true, true );~~
	660	+ $time = $wgLang->time( $rc->mAttribs['rc_timestamp'], true, true );
652	661	$rc->watched = $watched;
653	662	$rc->link = $clink;
654	663	$rc->timestamp = $time;
—	—	@@ -655,20 +664,22 @@
656	665
657	666	# Make "cur" and "diff" links. Do not use link(), it is too slow if
658	667	# called too many times (50% of CPU time on RecentChanges!).
	668	+ $thisOldid = $rc->mAttribs['rc_this_oldid'];
	669	+ $lastOldid = $rc->mAttribs['rc_last_oldid'];
659	670	if( $rc->unpatrolled ) {
660		~~- $rcIdQuery = array( 'rcid' => $rc_id );~~
	671	+ $rcIdQuery = array( 'rcid' => $rc->mAttribs['rc_id'] );
661	672	} else {
662	673	$rcIdQuery = array();
663	674	}
664		~~- $querycur = $curIdEq + array( 'diff' => '0', 'oldid' => $rc_this_oldid );~~
665		~~- $querydiff = $curIdEq + array( 'diff' => $rc_this_oldid, 'oldid' =>~~
666		~~- $rc_last_oldid ) + $rcIdQuery;~~
	675	+ $querycur = $curIdEq + array( 'diff' => '0', 'oldid' => $thisOldid );
	676	+ $querydiff = $curIdEq + array( 'diff' => $thisOldid, 'oldid' =>
	677	+ $lastOldid ) + $rcIdQuery;
667	678
668	679	if( !$showdifflinks ) {
669	680	$curLink = $this->message['cur'];
670	681	$diffLink = $this->message['diff'];
671		~~- } else if( in_array( $rc_type, array(RC_NEW,RC_LOG,RC_MOVE,RC_MOVE_OVER_REDIRECT) ) ) {~~
672		~~- if ( $rc_type != RC_NEW ) {~~
	682	+ } else if( in_array( $type, array( RC_NEW, RC_LOG, RC_MOVE, RC_MOVE_OVER_REDIRECT ) ) ) {
	683	+ if ( $type != RC_NEW ) {
673	684	$curLink = $this->message['cur'];
674	685	} else {
675	686	$curUrl = htmlspecialchars( $rc->getTitle()->getLinkUrl( $querycur ) );
—	—	@@ -683,21 +694,21 @@
684	695	}
685	696
686	697	# Make "last" link
687		~~- if( !$showdifflinks \|\| !$rc_last_oldid ) {~~
	698	+ if( !$showdifflinks \|\| !$lastOldid ) {
688	699	$lastLink = $this->message['last'];
689		~~- } else if( $rc_type == RC_LOG \|\| $rc_type == RC_MOVE \|\| $rc_type == RC_MOVE_OVER_REDIRECT ) {~~
	700	+ } else if( in_array( $type, array( RC_LOG, RC_MOVE, RC_MOVE_OVER_REDIRECT ) ) ) {
690	701	$lastLink = $this->message['last'];
691	702	} else {
692	703	$lastLink = $this->skin->linkKnown( $rc->getTitle(), $this->message['last'],
693		~~- array(), $curIdEq + array('diff' => $rc_this_oldid, 'oldid' => $rc_last_oldid) + $rcIdQuery );~~
	704	+ array(), $curIdEq + array('diff' => $thisOldid, 'oldid' => $lastOldid) + $rcIdQuery );
694	705	}
695	706
696	707	# Make user links
697		~~- if( $this->isDeleted($rc,Revision::DELETED_USER) ) {~~
698		~~- $rc->userlink = ' <span class="history-deleted">' . wfMsgHtml( 'rev-deleted-user' ) . '</span>';~~
	708	+ if( $this->isDeleted( $rc, Revision::DELETED_USER ) ) {
	709	+ $rc->userlink = ' <span class="history-deleted">' . wfMsgHtml( 'rev-deleted-user' ) . '</span>';
699	710	} else {
700		~~- $rc->userlink = $this->skin->userLink( $rc_user, $rc_user_text );~~
701		~~- $rc->usertalklink = $this->skin->userToolLinks( $rc_user, $rc_user_text );~~
	711	+ $rc->userlink = $this->skin->userLink( $rc->mAttribs['rc_user'], $rc->mAttribs['rc_user_text'] );
	712	+ $rc->usertalklink = $this->skin->userToolLinks( $rc->mAttribs['rc_user'], $rc->mAttribs['rc_user_text'] );
702	713	}
703	714
704	715	$rc->lastlink = $lastLink;
—	—	@@ -708,13 +719,13 @@
709	720	# Page moves go on their own line
710	721	$title = $rc->getTitle();
711	722	$secureName = $title->getPrefixedDBkey();
712		~~- if( $rc_type == RC_MOVE \|\| $rc_type == RC_MOVE_OVER_REDIRECT ) {~~
	723	+ if( $type == RC_MOVE \|\| $type == RC_MOVE_OVER_REDIRECT ) {
713	724	# Use an @ character to prevent collision with page names
714	725	$this->rc_cache['@@' . ($this->rcMoveIndex++)] = array($rc);
715	726	} else {
716	727	# Logs are grouped by type
717		~~- if( $rc_type == RC_LOG ){~~
718		~~- $secureName = SpecialPage::getTitleFor( 'Log', $rc_log_type )->getPrefixedDBkey();~~
	728	+ if( $type == RC_LOG ){
	729	+ $secureName = SpecialPage::getTitleFor( 'Log', $logType )->getPrefixedDBkey();
719	730	}
720	731	if( !isset( $this->rc_cache[$secureName] ) ) {
721	732	$this->rc_cache[$secureName] = array();
—	—	@@ -739,9 +750,9 @@
740	751	# Add the namespace and title of the block as part of the class
741	752	if ( $block[0]->mAttribs['rc_log_type'] ) {
742	753	# Log entry
743		~~- $classes = 'mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-log-' . $block[0]->mAttribs['rc_log_type'] . '-' . $block[0]->mAttribs['rc_title'] );~~
	754	+ $classes = 'mw-collapsible mw-collapsed mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-log-' . $block[0]->mAttribs['rc_log_type'] . '-' . $block[0]->mAttribs['rc_title'] );
744	755	} else {
745		~~- $classes = 'mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-ns' . $block[0]->mAttribs['rc_namespace'] . '-' . $block[0]->mAttribs['rc_title'] );~~
	756	+ $classes = 'mw-collapsible mw-collapsed mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-ns' . $block[0]->mAttribs['rc_namespace'] . '-' . $block[0]->mAttribs['rc_title'] );
746	757	}
747	758	$r = Html::openElement( 'table', array( 'class' => $classes ) ) .
748	759	Html::openElement( 'tr' );
—	—	@@ -804,23 +815,28 @@
805	816	$users = ' <span class="changedby">[' .
806	817	implode( $this->message['semicolon-separator'], $users ) . ']</span>';
807	818
808		~~- # ID for JS visibility toggle~~
809		~~- $jsid = $this->rcCacheIndex;~~
810		~~- # onclick handler to toggle hidden/expanded~~
811		~~- $toggleLink = "onclick='toggleVisibility($jsid); return false'";~~
812	819	# Title for <a> tags
813	820	$expandTitle = htmlspecialchars( wfMsg( 'rc-enhanced-expand' ) );
814	821	$closeTitle = htmlspecialchars( wfMsg( 'rc-enhanced-hide' ) );
815	822
816		~~- $tl = "<span id='mw-rc-openarrow-$jsid' class='mw-changeslist-expanded' style='visibility:hidden'><a href='#' $toggleLink title='$expandTitle'>" . $this->sideArrow() . "</a></span>";~~
817		~~- $tl .= "<span id='mw-rc-closearrow-$jsid' class='mw-changeslist-hidden' style='display:none'><a href='#' $toggleLink title='$closeTitle'>" . $this->downArrow() . "</a></span>";~~
818		~~- $r .= '<td class="mw-enhanced-rc">'.$tl.' ';~~
	823	+ $tl = "<span class='mw-collapsible-toggle'>"
	824	+ . "<span class='mw-rc-openarrow'>"
	825	+ . "<a href='#' title='$expandTitle'>{$this->sideArrow()}</a>"
	826	+ . "</span><span class='mw-rc-closearrow'>"
	827	+ . "<a href='#' title='$closeTitle'>{$this->downArrow()}</a>"
	828	+ . "</span></span>";
	829	+ $r .= "<td>$tl</td>";
819	830
820	831	# Main line
821		~~- $r .= $this->recentChangesFlags( $isnew, false, $unpatrolled, ' ', $bot );~~
	832	+ $r .= '<td class="mw-enhanced-rc">' . $this->recentChangesFlags( array(
	833	+ 'newpage' => $isnew,
	834	+ 'minoredit' => false,
	835	+ 'unpatrolled' => $unpatrolled,
	836	+ 'botedit' => $bot ,
	837	+ ) );
822	838
823	839	# Timestamp
824		~~- $r .= ' '.$block[0]->timestamp.' </td><td style="padding:0px;">';~~
	840	+ $r .= ' '.$block[0]->timestamp.' </td><td>';
825	841
826	842	# Article link
827	843	if( $namehidden ) {
—	—	@@ -908,39 +924,36 @@
909	925	$r .= $users;
910	926	$r .= $this->numberofWatchingusers($block[0]->numberofWatchingusers);
911	927
912		~~- $r .= "</td></tr></table>\n";~~
913		-
914	928	# Sub-entries
915		~~- $r .= '<div id="mw-rc-subentries-'.$jsid.'" class="mw-changeslist-hidden">';~~
916		~~- $r .= '<table class="mw-enhanced-rc">';~~
917	929	foreach( $block as $rcObj ) {
918		~~- # Extract fields from DB into the function scope (rc_xxxx variables)~~
919		~~- // FIXME: Would be good to replace this extract() call with something~~
920		~~- // that explicitly initializes variables.~~
921	930	# Classes to apply -- TODO implement
922	931	$classes = array();
923		~~- extract( $rcObj->mAttribs );~~
	932	+ $type = $rcObj->mAttribs['rc_type'];
924	933
925	934	#$r .= '<tr><td valign="top">'.$this->spacerArrow();
926		~~- $r .= '<tr><td style="vertical-align:top;font-family:monospace; padding:0px;">';~~
927		~~- $r .= $this->spacerIndent() . $this->spacerIndent();~~
928		~~- $r .= $this->recentChangesFlags( $rc_new, $rc_minor, $rcObj->unpatrolled, ' ', $rc_bot );~~
929		~~- $r .= ' </td><td style="vertical-align:top; padding:0px;"><span style="font-family:monospace">';~~
	935	+ $r .= '<tr><td></td><td class="mw-enhanced-rc">';
	936	+ $r .= $this->recentChangesFlags( array(
	937	+ 'newpage' => $rcObj->mAttribs['rc_new'],
	938	+ 'minoredit' => $rcObj->mAttribs['rc_minor'],
	939	+ 'unpatrolled' => $rcObj->unpatrolled,
	940	+ 'botedit' => $rcObj->mAttribs['rc_bot'],
	941	+ ) );
	942	+ $r .= ' </td><td class="mw-enhanced-rc-nested"><span class="mw-enhanced-rc-time">';
930	943
931	944	$params = $queryParams;
932	945
933		~~- if( $rc_this_oldid != 0 ) {~~
934		~~- $params['oldid'] = $rc_this_oldid;~~
	946	+ if( $rcObj->mAttribs['rc_this_oldid'] != 0 ) {
	947	+ $params['oldid'] = $rcObj->mAttribs['rc_this_oldid'];
935	948	}
936	949
937	950	# Log timestamp
938		~~- if( $rc_type == RC_LOG ) {~~
	951	+ if( $type == RC_LOG ) {
939	952	$link = $rcObj->timestamp;
940	953	# Revision link
941	954	} else if( !ChangesList::userCan($rcObj,Revision::DELETED_TEXT) ) {
942	955	$link = '<span class="history-deleted">'.$rcObj->timestamp.'</span> ';
943	956	} else {
944		~~- if ( $rcObj->unpatrolled && $rc_type == RC_NEW) {~~
	957	+ if ( $rcObj->unpatrolled && $type == RC_NEW) {
945	958	$params['rcid'] = $rcObj->mAttribs['rc_id'];
946	959	}
947	960
—	—	@@ -956,7 +969,7 @@
957	970	}
958	971	$r .= $link . '</span>';
959	972
960		~~- if ( !$rc_type == RC_LOG \|\| $rc_type == RC_NEW ) {~~
	973	+ if ( !$type == RC_LOG \|\| $type == RC_NEW ) {
961	974	$r .= ' (';
962	975	$r .= $rcObj->curlink;
963	976	$r .= $this->message['pipe-separator'];
—	—	@@ -966,9 +979,10 @@
967	980	$r .= ' . . ';
968	981
969	982	# Character diff
970		~~- if( $wgRCShowChangedSize ) {~~
971		~~- $r .= ( $rcObj->getCharacterDifference() == '' ? '' : $rcObj->getCharacterDifference() . ' . . ' ) ;~~
	983	+ if( $wgRCShowChangedSize && $rcObj->getCharacterDifference() ) {
	984	+ $r .= $rcObj->getCharacterDifference() . ' . . ' ;
972	985	}
	986	+
973	987	# User links
974	988	$r .= $rcObj->userlink;
975	989	$r .= $rcObj->usertalklink;
—	—	@@ -983,7 +997,7 @@
984	998
985	999	$r .= "</td></tr>\n";
986	1000	}
987		~~- $r .= "</table></div>\n";~~
	1001	+ $r .= "</table>\n";
988	1002
989	1003	$this->rcCacheIndex++;
990	1004
—	—	@@ -1036,14 +1050,6 @@
1037	1051	}
1038	1052
1039	1053	/**
1040		~~- * Add a set of spaces~~
1041		~~- * @return String: HTML <td> tag~~
1042		~~- */~~
1043		~~- protected function spacerIndent() {~~
1044		~~- return '     ';~~
1045		~~- }~~
1046		-
1047		- /**
1048	1054	* Enhanced RC ungrouped line.
1049	1055	* @return String: a HTML formated line (generated using $r)
1050	1056	*/
—	—	@@ -1051,35 +1057,36 @@
1052	1058	global $wgRCShowChangedSize;
1053	1059
1054	1060	wfProfileIn( __METHOD__ );
	1061	+ $query['curid'] = $rcObj->mAttribs['rc_cur_id'];
1055	1062
1056		~~- # Extract fields from DB into the function scope (rc_xxxx variables)~~
1057		~~- // FIXME: Would be good to replace this extract() call with something~~
1058		~~- // that explicitly initializes variables.~~
1059		~~- // TODO implement~~
1060		~~- extract( $rcObj->mAttribs );~~
1061		~~- $query['curid'] = $rc_cur_id;~~
1062		-
1063		~~- if( $rc_log_type ) {~~
	1063	+ $type = $rcObj->mAttribs['rc_type'];
	1064	+ $logType = $rcObj->mAttribs['rc_log_type'];
	1065	+ if( $logType ) {
1064	1066	# Log entry
1065		~~- $classes = 'mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-log-' . $rc_log_type . '-' . $rcObj->mAttribs['rc_title'] );~~
	1067	+ $classes = 'mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-log-' . $logType . '-' . $rcObj->mAttribs['rc_title'] );
1066	1068	} else {
1067	1069	$classes = 'mw-enhanced-rc ' . Sanitizer::escapeClass( 'mw-changeslist-ns' . $rcObj->mAttribs['rc_namespace'] . '-' . $rcObj->mAttribs['rc_title'] );
1068	1070	}
1069	1071	$r = Html::openElement( 'table', array( 'class' => $classes ) ) .
1070	1072	Html::openElement( 'tr' );
1071	1073
1072		~~- $r .= '<td class="mw-enhanced-rc">' . $this->spacerArrow() . ' ';~~
	1074	+ $r .= '<td class="mw-enhanced-rc">' . $this->spacerArrow();
1073	1075	# Flag and Timestamp
1074		~~- if( $rc_type == RC_MOVE \|\| $rc_type == RC_MOVE_OVER_REDIRECT ) {~~
	1076	+ if( $type == RC_MOVE \|\| $type == RC_MOVE_OVER_REDIRECT ) {
1075	1077	$r .= '    '; // 4 flags -> 4 spaces
1076	1078	} else {
1077		~~- $r .= $this->recentChangesFlags( $rc_type == RC_NEW, $rc_minor, $rcObj->unpatrolled, ' ', $rc_bot );~~
	1079	+ $r .= $this->recentChangesFlags( array(
	1080	+ 'newpage' => $type == RC_NEW,
	1081	+ 'minoredit' => $rcObj->mAttribs['rc_minor'],
	1082	+ 'unpatrolled' => $rcObj->unpatrolled,
	1083	+ 'botedit' => $rcObj->mAttribs['rc_bot'],
	1084	+ ) );
1078	1085	}
1079		~~- $r .= ' '.$rcObj->timestamp.' </td><td style="padding:0px;">';~~
	1086	+ $r .= ' '.$rcObj->timestamp.' </td><td>';
1080	1087	# Article or log link
1081		~~- if( $rc_log_type ) {~~
1082		~~- $logtitle = Title::newFromText( "Log/$rc_log_type", NS_SPECIAL );~~
1083		~~- $logname = LogPage::logName( $rc_log_type );~~
	1088	+ if( $logType ) {
	1089	+ $logtitle = Title::newFromText( "Log/$logType", NS_SPECIAL );
	1090	+ $logname = LogPage::logName( $logType );
1084	1091	$r .= '(' . $this->skin->link(
1085	1092	$logtitle,
1086	1093	$logname,
—	—	@@ -1091,7 +1098,7 @@
1092	1099	$this->insertArticleLink( $r, $rcObj, $rcObj->unpatrolled, $rcObj->watched );
1093	1100	}
1094	1101	# Diff and hist links
1095		~~- if ( $rc_type != RC_LOG ) {~~
	1102	+ if ( $type != RC_LOG ) {
1096	1103	$r .= ' ('. $rcObj->difflink . $this->message['pipe-separator'];
1097	1104	$query['action'] = 'history';
1098	1105	$r .= $this->skin->link(
—	—	@@ -1110,12 +1117,12 @@
1111	1118	# User/talk
1112	1119	$r .= ' '.$rcObj->userlink . $rcObj->usertalklink;
1113	1120	# Log action (if any)
1114		~~- if( $rc_log_type ) {~~
	1121	+ if( $logType ) {
1115	1122	if( $this->isDeleted($rcObj,LogPage::DELETED_ACTION) ) {
1116	1123	$r .= ' <span class="history-deleted">' . wfMsgHtml('rev-deleted-event') . '</span>';
1117	1124	} else {
1118		~~- $r .= ' ' . LogPage::actionText( $rc_log_type, $rc_log_action, $rcObj->getTitle(),~~
1119		~~- $this->skin, LogPage::extractParams($rc_params), true, true );~~
	1125	+ $r .= ' ' . LogPage::actionText( $logType, $rcObj->mAttribs['rc_log_action'], $rcObj->getTitle(),
	1126	+ $this->skin, LogPage::extractParams( $rcObj->mAttribs['rc_params'] ), true, true );
1120	1127	}
1121	1128	}
1122	1129	$this->insertComment( $r, $rcObj );
Index: trunk/phase3/resources/Resources.php
—	—	@@ -357,6 +357,10 @@
358	358	'scripts' => 'resources/mediawiki.special/mediawiki.special.preferences.js',
359	359	'styles' => 'resources/mediawiki.special/mediawiki.special.preferences.css',
360	360	),
	361	+ 'mediawiki.special.changeslist' => array(
	362	+ 'styles' => 'resources/mediawiki.special/mediawiki.special.changeslist.css',
	363	+ 'dependencies' => array( 'jquery.makeCollapsible' ),
	364	+ ),
361	365	'mediawiki.special.search' => array(
362	366	'scripts' => 'resources/mediawiki.special/mediawiki.special.search.js',
363	367	),
—	—	@@ -447,10 +451,6 @@
448	452	'scripts' => 'skins/common/edit.js',
449	453	'dependencies' => 'mediawiki.legacy.wikibits',
450	454	),
451		~~- 'mediawiki.legacy.enhancedchanges' => array(~~
452		~~- 'scripts' => 'skins/common/enhancedchanges.js',~~
453		~~- 'dependencies' => 'mediawiki.legacy.wikibits',~~
454		~~- ),~~
455	455	'mediawiki.legacy.history' => array(
456	456	'scripts' => 'skins/common/history.js',
457	457	'dependencies' => 'mediawiki.legacy.wikibits',
Index: trunk/phase3/resources/mediawiki.special/mediawiki.special.changeslist.css
—	—	@@ -0,0 +1,48 @@
	2	+/**
	3	+ * Styling for Special:Watchlist and Special:RecentChanges
	4	+ */
	5	+
	6	+table.mw-enhanced-rc {
	7	+ background: none;
	8	+ border:0;
	9	+ border-spacing:0;
	10	+}
	11	+
	12	+table.mw-enhanced-rc th, table.mw-enhanced-rc td {
	13	+ padding:0;
	14	+ vertical-align:top;
	15	+}
	16	+
	17	+td.mw-enhanced-rc {
	18	+ white-space:nowrap;
	19	+ font-family:monospace;
	20	+}
	21	+
	22	+.mw-enhanced-rc-time {
	23	+ font-family: monospace;
	24	+}
	25	+
	26	+table.mw-enhanced-rc td.mw-enhanced-rc-nested {
	27	+ padding-left: 1em;
	28	+}
	29	+
	30	+/* Show/hide arrows in enhanced changeslist */
	31	+.mw-enhanced-rc .collapsible-expander {
	32	+ float: none;
	33	+}
	34	+
	35	+/* If JS is disabled, the arrow is still needed
	36	+ for spacing, but ideally shouldn't be shown */
	37	+.mw-enhanced-rc .mw-rc-openarrow {
	38	+ visibility: hidden;
	39	+}
	40	+
	41	+.mw-enhanced-rc.mw-made-collapsible .mw-rc-openarrow,
	42	+.mw-enhanced-rc .mw-rc-closearrow {
	43	+ visibility: visible;
	44	+ display: none;
	45	+}
	46	+.mw-enhanced-rc.mw-made-collapsible .mw-collapsible-toggle-collapsed .mw-rc-openarrow,
	47	+.mw-enhanced-rc.mw-made-collapsible .mw-collapsible-toggle-expanded .mw-rc-closearrow {
	48	+ display: inline;
	49	+}
Property changes on: trunk/phase3/resources/mediawiki.special/mediawiki.special.changeslist.css
___________________________________________________________________
Added: svn:eol-style
1	50	+ native

Sign-offs

User	Flag	Date
Aaron Schulz	inspected	23:32, 19 April 2011
Aaron Schulz	tested	23:32, 19 April 2011

Follow-up revisions

Revision	Commit summary	Author	Date
r79084	Follow-up r79083: Fix notices on TW.	happy-melon	19:50, 27 December 2010
r89901	Remove resize/arrow cursors from jquery.makeCollapsible...	krinkle	00:17, 12 June 2011

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r44421	Revert r44119 "Remove extract() comment"...	brion	19:41, 10 December 2008

Comments

#Comment by Krinkle (talk | contribs) 19:47, 27 December 2010

Errors in translatewiki debug:

PHP Notice:  Undefined index:  bot in /www/w/includes/ChangesList.php on line 133

#Comment by Raymond (talk | contribs) 19:47, 27 December 2010

PHP Notice: Undefined index: bot in /www/w/includes/ChangesList.php on line 133

#Comment by Happy-melon (talk | contribs) 19:49, 27 December 2010

Does it give stack traces?

#Comment by Raymond (talk | contribs) 19:50, 27 December 2010

No. but this one too:

PHP Notice:  Undefined index:  minor in /www/w/includes/ChangesList.php on line 133

#Comment by Krinkle (talk | contribs) 19:51, 27 December 2010

No, it's means it doesn't check if $flags[$flag] is set before it references it.

#Comment by Happy-melon (talk | contribs) 19:53, 27 December 2010

I seem to have written a one-to-one map :D. Fixed in r79084.

#Comment by Aaron Schulz (talk | contribs) 23:23, 19 April 2011

I'm getting a vertical double-arrow over the blue triangle icons in FF4 :/

#Comment by Aaron Schulz (talk | contribs) 23:23, 19 April 2011

As the cursor, to be clear. Collapse works though.

#Comment by Happy-melon (talk | contribs) 17:10, 20 April 2011

That's deliberate, it's part of Krinkle's new collapsible plugin (r78914 and friends).

#Comment by Aaron Schulz (talk | contribs) 17:14, 20 April 2011

That absolutely has to change. Can you override it in the RC css?

#Comment by Happy-melon (talk | contribs) 17:16, 20 April 2011

Why only change it for this instance, if it needs to be changed?

#Comment by Aaron Schulz (talk | contribs) 17:24, 20 April 2011

As long as I don't see arrow icons that have that double-arrow cursor over them...

#Comment by Krinkle (talk | contribs) 00:19, 12 June 2011

Fixed in r89901.

I wasn't sure what you meant here, since the cursor looked fine here..

It appears the cursor icon browsers have implement for n-resize and s-resize varies a lot. I've removed it for now.

Status & tagging log

00:17, 12 June 2011 Krinkle (talk | contribs) changed the status of r79083 [removed: new added: ok]
17:10, 20 April 2011 Happy-melon (talk | contribs) changed the status of r79083 [removed: fixme added: new]
23:23, 19 April 2011 Aaron Schulz (talk | contribs) changed the status of r79083 [removed: new added: fixme]
19:53, 27 December 2010 Happy-melon (talk | contribs) changed the status of r79083 [removed: fixme added: new]
19:47, 27 December 2010 Raymond (talk | contribs) changed the status of r79083 [removed: new added: fixme]