r96842 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r96841‎ \| r96842 \| r96843 >
Date:	14:42, 12 September 2011
Author:	reedy
Status:	ok
Tags:
Comment:	REL1_18: MFT r93288, r94212, r96261, r96263, r96384
Modified paths:	/branches/REL1_18/phase3/includes/OutputPage.php (modified) (history) /branches/REL1_18/phase3/includes/SkinLegacy.php (modified) (history) /branches/REL1_18/phase3/includes/filerepo/LocalFile.php (modified) (history) /branches/REL1_18/phase3/includes/resourceloader/ResourceLoaderUserOptionsModule.php (modified) (history) /branches/REL1_18/phase3/skins/CologneBlue.php (modified) (history) /branches/REL1_18/phase3/skins/Simple.php (modified) (history) /branches/REL1_18/phase3/skins/Standard.php (modified) (history) /branches/REL1_18/phase3/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js (modified) (history)

Diff [purge]

Index: branches/REL1_18/phase3/skins/Simple.php
—	—	@@ -27,12 +27,12 @@
28	28	$out->addModuleStyles( 'skins.simple' );
29	29
30	30	/* Add some userprefs specific CSS styling */
31		~~- global $wgUser, $wgContLang;~~
	31	+ global $wgUser;
32	32	$rules = array();
33	33	$underline = "";
34	34
35	35	if ( $wgUser->getOption( 'underline' ) < 2 ) {
36		~~- $underline = "text-decoration: " . $wgUser->getOption( 'underline' ) ? 'underline' : 'none' . ";";~~
	36	+ $underline = "text-decoration: " . $wgUser->getOption( 'underline' ) ? 'underline !important' : 'none' . ";";
37	37	}
38	38
39	39	/* Also inherits from resourceloader */
—	—	@@ -42,10 +42,7 @@
43	43	$rules[] = "a.stub:after { $underline; }";
44	44	}
45	45	$style = implode( "\n", $rules );
46		~~- if ( $wgContLang->getDir() === 'rtl' ) {~~
47		~~- $style = CSSJanus::transform( $style, true, false );~~
48		~~- }~~
49		~~- $out->addInlineStyle( $style );~~
	46	+ $out->addInlineStyle( $style, /* flip css if RTL */true );
50	47
51	48	}
52	49	}
Index: branches/REL1_18/phase3/skins/CologneBlue.php
—	—	@@ -27,24 +27,24 @@
28	28	$rules = array();
29	29
30	30	if ( 2 == $qb ) { # Right
31		~~- $rules[] = "#quickbar { position: absolute; right: 4px; }";~~
32		~~- $rules[] = "#article { margin-left: 4px; margin-right: 148px; }";~~
	31	+ $rules[] = "/* @noflip */#quickbar { position: absolute; right: 4px; }";
	32	+ $rules[] = "/* @noflip */#article { margin-left: 4px; margin-right: 148px; }";
33	33	} elseif ( 1 == $qb ) {
34		~~- $rules[] = "#quickbar { position: absolute; left: 4px; }";~~
35		~~- $rules[] = "#article { margin-left: 148px; margin-right: 4px; }";~~
	34	+ $rules[] = "/* @noflip */#quickbar { position: absolute; left: 4px; }";
	35	+ $rules[] = "/* @noflip */#article { margin-left: 148px; margin-right: 4px; }";
36	36	} elseif ( 3 == $qb ) { # Floating left
37		~~- $rules[] = "#quickbar { position:absolute; left:4px }";~~
38		~~- $rules[] = "#topbar { margin-left: 148px }";~~
39		~~- $rules[] = "#article { margin-left:148px; margin-right: 4px; }";~~
40		~~- $rules[] = "body>#quickbar { position:fixed; left:4px; top:4px; overflow:auto ;bottom:4px;}"; # Hides from IE~~
	37	+ $rules[] = "/* @noflip */#quickbar { position:absolute; left:4px }";
	38	+ $rules[] = "/* @noflip */#topbar { margin-left: 148px }";
	39	+ $rules[] = "/* @noflip */#article { margin-left:148px; margin-right: 4px; }";
	40	+ $rules[] = "/* @noflip */body>#quickbar { position:fixed; left:4px; top:4px; overflow:auto; bottom:4px;}"; # Hides from IE
41	41	} elseif ( 4 == $qb ) { # Floating right
42		~~- $rules[] = "#quickbar { position: fixed; right: 4px; }";~~
43		~~- $rules[] = "#topbar { margin-right: 148px }";~~
44		~~- $rules[] = "#article { margin-right: 148px; margin-left: 4px; }";~~
45		~~- $rules[] = "body>#quickbar { position: fixed; right: 4px; top: 4px; overflow: auto ;bottom:4px;}"; # Hides from IE~~
	42	+ $rules[] = "/* @noflip */#quickbar { position: fixed; right: 4px; }";
	43	+ $rules[] = "/* @noflip */#topbar { margin-right: 148px }";
	44	+ $rules[] = "/* @noflip */#article { margin-right: 148px; margin-left: 4px; }";
	45	+ $rules[] = "/* @noflip */body>#quickbar { position: fixed; right: 4px; top: 4px; overflow: auto; bottom:4px;}"; # Hides from IE
46	46	}
47	47	$style = implode( "\n", $rules );
48		~~- $out->addInlineStyle( $style );~~
	48	+ $out->addInlineStyle( $style, /* flip css if RTL */true );
49	49	}
50	50
51	51	}
—	—	@@ -221,10 +221,12 @@
222	222	}
223	223	$s .= "\n<h6>" . htmlspecialchars( $h ) . "</h6>";
224	224	}
225		~~- foreach ( $browseLinks as $link ) {~~
226		~~- if ( $link['text'] != '-' ) {~~
227		~~- $s .= "<a href=\"{$link['href']}\">" .~~
228		~~- htmlspecialchars( $link['text'] ) . '</a>' . $sep;~~
	225	+ if( is_array( $browseLinks ) ) {
	226	+ foreach ( $browseLinks as $link ) {
	227	+ if ( $link['text'] != '-' ) {
	228	+ $s .= "<a href=\"{$link['href']}\">" .
	229	+ htmlspecialchars( $link['text'] ) . '</a>' . $sep;
	230	+ }
229	231	}
230	232	}
231	233	$barnumber++;
Index: branches/REL1_18/phase3/skins/Standard.php
—	—	@@ -26,21 +26,21 @@
27	27	$rules = array();
28	28
29	29	if ( 2 == $qb ) { # Right
30		~~- $rules[] = "#quickbar { position: absolute; top: 4px; right: 4px; border-left: 2px solid #000000; }";~~
31		~~- $rules[] = "#article, #mw-data-after-content { margin-left: 4px; margin-right: 152px; }";~~
	30	+ $rules[] = "/* @noflip */#quickbar { position: absolute; top: 4px; right: 4px; border-left: 2px solid #000000; }";
	31	+ $rules[] = "/* @noflip /#article, / @noflip */#mw-data-after-content { margin-left: 4px; margin-right: 152px; }";
32	32	} elseif ( 1 == $qb \|\| 3 == $qb ) {
33		~~- $rules[] = "#quickbar { position: absolute; top: 4px; left: 4px; border-right: 1px solid gray; }";~~
34		~~- $rules[] = "#article, #mw-data-after-content { margin-left: 152px; margin-right: 4px; }";~~
	33	+ $rules[] = "/* @noflip */#quickbar { position: absolute; top: 4px; left: 4px; border-right: 1px solid gray; }";
	34	+ $rules[] = "/* @noflip /#article, / @noflip */#mw-data-after-content { margin-left: 152px; margin-right: 4px; }";
35	35	if( 3 == $qb ) {
36		~~- $rules[] = "#quickbar { position: fixed; padding: 4px; }";~~
	36	+ $rules[] = "/* @noflip */#quickbar { position: fixed; padding: 4px; }";
37	37	}
38	38	} elseif ( 4 == $qb ) {
39		~~- $rules[] = "#quickbar { position: fixed; right: 0px; top: 0px; padding: 4px;}";~~
40		~~- $rules[] = "#quickbar { border-right: 1px solid gray; }";~~
41		~~- $rules[] = "#article, #mw-data-after-content { margin-right: 152px; margin-left: 4px; }";~~
	39	+ $rules[] = "/* @noflip */#quickbar { position: fixed; right: 0px; top: 0px; padding: 4px;}";
	40	+ $rules[] = "/* @noflip */#quickbar { border-right: 1px solid gray; }";
	41	+ $rules[] = "/* @noflip /#article, / @noflip */#mw-data-after-content { margin-right: 152px; margin-left: 4px; }";
42	42	}
43	43	$style = implode( "\n", $rules );
44		~~- $out->addInlineStyle( $style );~~
	44	+ $out->addInlineStyle( $style, /* flip css if RTL */true );
45	45	}
46	46
47	47	}
Index: branches/REL1_18/phase3/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
—	—	@@ -368,4 +368,87 @@
369	369	}
370	370	);
371	371
	372	+test( 'data-sort-value attribute, when available, should override sorting position', function() {
	373	+ var $table, data;
	374	+
	375	+ // Simple example, one without data-sort-value which should be sorted at it's text.
	376	+ $table = $(
	377	+ '<table class="sortable"><thead><tr><th>Data</th></tr></thead>' +
	378	+ '<tbody>' +
	379	+ '<tr><td>Cheetah</td></tr>' +
	380	+ '<tr><td data-sort-value="Apple">Bird</td></tr>' +
	381	+ '<tr><td data-sort-value="Bananna">Ferret</td></tr>' +
	382	+ '<tr><td data-sort-value="Drupe">Elephant</td></tr>' +
	383	+ '<tr><td data-sort-value="Cherry">Dolphin</td></tr>' +
	384	+ '</tbody></table>'
	385	+ );
	386	+ $table.tablesorter().find( '.headerSort:eq(0)' ).click();
	387	+
	388	+ data = [];
	389	+ $table.find( 'tbody > tr' ).each( function( i, tr ) {
	390	+ $( tr ).find( 'td' ).each( function( i, td ) {
	391	+ data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } );
	392	+ });
	393	+ });
	394	+
	395	+ deepEqual( data, [
	396	+ {
	397	+ "data": "Apple",
	398	+ "text": "Bird"
	399	+ }, {
	400	+ "data": "Bananna",
	401	+ "text": "Ferret"
	402	+ }, {
	403	+ "data": undefined,
	404	+ "text": "Cheetah"
	405	+ }, {
	406	+ "data": "Cherry",
	407	+ "text": "Dolphin"
	408	+ }, {
	409	+ "data": "Drupe",
	410	+ "text": "Elephant"
	411	+ }
	412	+ ] );
	413	+
	414	+ // Another example
	415	+ $table = $(
	416	+ '<table class="sortable"><thead><tr><th>Data</th></tr></thead>' +
	417	+ '<tbody>' +
	418	+ '<tr><td>D</td></tr>' +
	419	+ '<tr><td data-sort-value="E">A</td></tr>' +
	420	+ '<tr><td>B</td></tr>' +
	421	+ '<tr><td>G</td></tr>' +
	422	+ '<tr><td data-sort-value="F">C</td></tr>' +
	423	+ '</tbody></table>'
	424	+ );
	425	+ $table.tablesorter().find( '.headerSort:eq(0)' ).click();
	426	+
	427	+ data = [];
	428	+ $table.find( 'tbody > tr' ).each( function( i, tr ) {
	429	+ $( tr ).find( 'td' ).each( function( i, td ) {
	430	+ data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } );
	431	+ });
	432	+ });
	433	+
	434	+ deepEqual( data, [
	435	+ {
	436	+ "data": undefined,
	437	+ "text": "B"
	438	+ }, {
	439	+ "data": undefined,
	440	+ "text": "D"
	441	+ }, {
	442	+ "data": "E",
	443	+ "text": "A"
	444	+ }, {
	445	+ "data": "F",
	446	+ "text": "C"
	447	+ }, {
	448	+ "data": undefined,
	449	+ "text": "G"
	450	+ }
	451	+ ] );
	452	+
	453	+});
	454	+
372	455	})();
Index: branches/REL1_18/phase3/includes/filerepo/LocalFile.php
—	—	@@ -655,6 +655,11 @@
656	656	$hashedName = md5( $this->getName() );
657	657	$oldKey = $this->repo->getSharedCacheKey( 'oldfile', $hashedName );
658	658
	659	+ // Must purge thumbnails for old versions too! bug 30192
	660	+ foreach( $this->getHistory() as $oldFile ) {
	661	+ $oldFile->purgeThumbnails();
	662	+ }
	663	+
659	664	if ( $oldKey ) {
660	665	$wgMemc->delete( $oldKey );
661	666	}
Property changes on: branches/REL1_18/phase3/includes/filerepo/LocalFile.php
___________________________________________________________________
Modified: svn:mergeinfo
662	667	Merged /trunk/phase3/includes/filerepo/LocalFile.php:r93288,94212,96261,96263,96384
Index: branches/REL1_18/phase3/includes/OutputPage.php
—	—	@@ -2955,8 +2955,13 @@
2956	2956	/**
2957	2957	* Adds inline CSS styles
2958	2958	* @param $style_css Mixed: inline CSS
	2959	+ * @param $flip Boolean: Whether to flip the CSS if needed
2959	2960	*/
2960		~~- public function addInlineStyle( $style_css ){~~
	2961	+ public function addInlineStyle( $style_css, $flip = false ) {
	2962	+ if( $flip && $this->getLang()->isRTL() ) {
	2963	+ # If wanted, and the interface is right-to-left, flip the CSS
	2964	+ $style_css = CSSJanus::transform( $style_css, true, false );
	2965	+ }
2961	2966	$this->mInlineStyles .= Html::inlineStyle( $style_css );
2962	2967	}
2963	2968
Property changes on: branches/REL1_18/phase3/includes/OutputPage.php
___________________________________________________________________
Modified: svn:mergeinfo
2964	2969	Merged /trunk/phase3/includes/OutputPage.php:r93288,94212,96261,96263,96384
Index: branches/REL1_18/phase3/includes/resourceloader/ResourceLoaderUserOptionsModule.php
—	—	@@ -93,7 +93,7 @@
94	94	$rules = array();
95	95	if ( $options['underline'] < 2 ) {
96	96	$rules[] = "a { text-decoration: " .
97		~~- ( $options['underline'] ? 'underline' : 'none' ) . "; }";~~
	97	+ ( $options['underline'] ? 'underline !important' : 'none' ) . "; }";
98	98	}
99	99	if ( $options['highlightbroken'] ) {
100	100	$rules[] = "a.new, #quickbar a.new { color: #ba0000; }\n";
Index: branches/REL1_18/phase3/includes/SkinLegacy.php
—	—	@@ -248,7 +248,7 @@
249	249
250	250	$s = array(
251	251	$this->getSkin()->mainPageLink(),
252		~~- $this->getSkin()->specialLink( 'Recentchanges' )~~
	252	+ Linker::specialLink( 'Recentchanges' )
253	253	);
254	254
255	255	if ( $wgOut->isArticleRelated() ) {
—	—	@@ -501,7 +501,7 @@
502	502	}
503	503
504	504	if ( 'history' == $action \|\| isset( $diff ) \|\| isset( $oldid ) ) {
505		~~- $s[] .= $this->getSkin()->link(~~
	505	+ $s[] .= Linker::link(
506	506	$this->getSkin()->getTitle(),
507	507	wfMsg( 'currentrev' ),
508	508	array(),
—	—	@@ -514,7 +514,7 @@
515	515	# do not show "You have new messages" text when we are viewing our
516	516	# own talk page
517	517	if ( !$this->getSkin()->getTitle()->equals( $wgUser->getTalkPage() ) ) {
518		~~- $tl = $this->getSkin()->link(~~
	518	+ $tl = Linker::link(
519	519	$wgUser->getTalkPage(),
520	520	wfMsgHtml( 'newmessageslink' ),
521	521	array(),
—	—	@@ -522,7 +522,7 @@
523	523	array( 'known', 'noclasses' )
524	524	);
525	525
526		~~- $dl = $this->getSkin()->link(~~
	526	+ $dl = Linker::link(
527	527	$wgUser->getTalkPage(),
528	528	wfMsgHtml( 'newmessagesdifflink' ),
529	529	array(),
—	—	@@ -611,7 +611,7 @@
612	612	$t = wfMsg( 'viewsource' );
613	613	}
614	614
615		~~- $s = $this->getSkin()->link(~~
	615	+ $s = Linker::link(
616	616	$this->getSkin()->getTitle(),
617	617	$t,
618	618	array(),
—	—	@@ -631,7 +631,7 @@
632	632	if ( $this->getSkin()->getTitle()->getArticleId() && ( !$diff ) && $wgUser->isAllowed( 'delete' ) ) {
633	633	$t = wfMsg( 'deletethispage' );
634	634
635		~~- $s = $this->getSkin()->link(~~
	635	+ $s = Linker::link(
636	636	$this->getSkin()->getTitle(),
637	637	$t,
638	638	array(),
—	—	@@ -659,7 +659,7 @@
660	660	$query = array( 'action' => 'protect' );
661	661	}
662	662
663		~~- $s = $this->getSkin()->link(~~
	663	+ $s = Linker::link(
664	664	$this->getSkin()->getTitle(),
665	665	$text,
666	666	array(),
—	—	@@ -697,7 +697,7 @@
698	698	$id = 'mw-watch-link' . $this->mWatchLinkNum;
699	699	}
700	700
701		~~- $s = $this->getSkin()->link(~~
	701	+ $s = Linker::link(
702	702	$title,
703	703	$text,
704	704	array( 'id' => $id ),
—	—	@@ -713,7 +713,7 @@
714	714
715	715	function moveThisPage() {
716	716	if ( $this->getSkin()->getTitle()->quickUserCan( 'move' ) ) {
717		~~- return $this->getSkin()->link(~~
	717	+ return Linker::link(
718	718	SpecialPage::getTitleFor( 'Movepage' ),
719	719	wfMsg( 'movethispage' ),
720	720	array(),
—	—	@@ -727,7 +727,7 @@
728	728	}
729	729
730	730	function historyLink() {
731		~~- return $this->getSkin()->link(~~
	731	+ return Linker::link(
732	732	$this->getSkin()->getTitle(),
733	733	wfMsgHtml( 'history' ),
734	734	array( 'rel' => 'archives' ),
—	—	@@ -736,7 +736,7 @@
737	737	}
738	738
739	739	function whatLinksHere() {
740		~~- return $this->getSkin()->link(~~
	740	+ return Linker::link(
741	741	SpecialPage::getTitleFor( 'Whatlinkshere', $this->getSkin()->getTitle()->getPrefixedDBkey() ),
742	742	wfMsgHtml( 'whatlinkshere' ),
743	743	array(),
—	—	@@ -746,7 +746,7 @@
747	747	}
748	748
749	749	function userContribsLink() {
750		~~- return $this->getSkin()->link(~~
	750	+ return Linker::link(
751	751	SpecialPage::getTitleFor( 'Contributions', $this->getSkin()->getTitle()->getDBkey() ),
752	752	wfMsgHtml( 'contributions' ),
753	753	array(),
—	—	@@ -756,7 +756,7 @@
757	757	}
758	758
759	759	function emailUserLink() {
760		~~- return $this->getSkin()->link(~~
	760	+ return Linker::link(
761	761	SpecialPage::getTitleFor( 'Emailuser', $this->getSkin()->getTitle()->getDBkey() ),
762	762	wfMsg( 'emailuser' ),
763	763	array(),
—	—	@@ -771,7 +771,7 @@
772	772	if ( !$wgOut->isArticleRelated() ) {
773	773	return '(' . wfMsg( 'notanarticle' ) . ')';
774	774	} else {
775		~~- return $this->getSkin()->link(~~
	775	+ return Linker::link(
776	776	SpecialPage::getTitleFor( 'Recentchangeslinked', $this->getSkin()->getTitle()->getPrefixedDBkey() ),
777	777	wfMsg( 'recentchangeslinked-toolbox' ),
778	778	array(),
—	—	@@ -832,7 +832,7 @@
833	833	$text = wfMsg( 'talkpage' );
834	834	}
835	835
836		~~- $s = $this->getSkin()->link( $link, $text, array(), array(), $linkOptions );~~
	836	+ $s = Linker::link( $link, $text, array(), array(), $linkOptions );
837	837
838	838	return $s;
839	839	}
—	—	@@ -855,7 +855,7 @@
856	856	$title = $this->getSkin()->getTitle()->getTalkPage();
857	857	}
858	858
859		~~- return $this->getSkin()->link(~~
	859	+ return Linker::link(
860	860	$title,
861	861	wfMsg( 'postcomment' ),
862	862	array(),
—	—	@@ -874,7 +874,7 @@
875	875	# Using an empty class attribute to avoid automatic setting of "external" class
876	876	return Linker::makeExternalLink( $wgUploadNavigationUrl, wfMsgHtml( 'upload' ), false, null, array( 'class' => '' ) );
877	877	} else {
878		~~- return $this->getSkin()->link(~~
	878	+ return Linker::link(
879	879	SpecialPage::getTitleFor( 'Upload' ),
880	880	wfMsgHtml( 'upload' ),
881	881	array(),
—	—	@@ -895,7 +895,7 @@
896	896	if ( $this->getSkin()->showIPinHeader() ) {
897	897	$name = wfGetIP();
898	898
899		~~- $talkLink = $this->getSkin()->link( $wgUser->getTalkPage(),~~
	899	+ $talkLink = Linker::link( $wgUser->getTalkPage(),
900	900	$wgLang->getNsText( NS_TALK ) );
901	901
902	902	$ret .= "$name ($talkLink)";
—	—	@@ -913,30 +913,30 @@
914	914	$loginlink = $wgUser->isAllowed( 'createaccount' )
915	915	? 'nav-login-createaccount'
916	916	: 'login';
917		~~- $ret .= "\n<br />" . $this->getSkin()->link(~~
	917	+ $ret .= "\n<br />" . Linker::link(
918	918	SpecialPage::getTitleFor( 'Userlogin' ),
919	919	wfMsg( $loginlink ), array(), $query
920	920	);
921	921	} else {
922	922	$returnTo = $this->getSkin()->getTitle()->getPrefixedDBkey();
923		~~- $talkLink = $this->getSkin()->link( $wgUser->getTalkPage(),~~
	923	+ $talkLink = Linker::link( $wgUser->getTalkPage(),
924	924	$wgLang->getNsText( NS_TALK ) );
925	925
926		~~- $ret .= $this->getSkin()->link( $wgUser->getUserPage(),~~
	926	+ $ret .= Linker::link( $wgUser->getUserPage(),
927	927	htmlspecialchars( $wgUser->getName() ) );
928	928	$ret .= " ($talkLink)<br />";
929	929	$ret .= $wgLang->pipeList( array(
930		~~- $this->getSkin()->link(~~
	930	+ Linker::link(
931	931	SpecialPage::getTitleFor( 'Userlogout' ), wfMsg( 'logout' ),
932	932	array(), array( 'returnto' => $returnTo )
933	933	),
934		~~- $this->getSkin()->specialLink( 'Preferences' ),~~
	934	+ Linker::specialLink( 'Preferences' ),
935	935	) );
936	936	}
937	937
938	938	$ret = $wgLang->pipeList( array(
939	939	$ret,
940		~~- $this->getSkin()->link(~~
	940	+ Linker::link(
941	941	Title::newFromText( wfMsgForContent( 'helppage' ) ),
942	942	wfMsg( 'help' )
943	943	),

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r93288	SkinLegacy:...	robin	17:20, 27 July 2011
r94212	(bug 30192) Old thumbnails not properly purged. Unlike the bug suggests, we d...	demon	23:29, 10 August 2011
r96261	* Mark underline css as !important. It should override other css (such as the...	robin	21:50, 4 September 2011
r96263	Fix r96261: don't add important if 'none', so it doesn't break underlining on...	robin	22:05, 4 September 2011
r96384	jquery.tablesorter.test: Add tests for data-sort-value...	krinkle	23:09, 6 September 2011

Status & tagging log

14:43, 12 September 2011 😂 (talk | contribs) changed the status of r96842 [removed: new added: ok]