r85080 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r85079‎ | r85080 | r85081 >
Date:19:09, 31 March 2011
Author:demon
Status:resolved (Comments)
Tags:
Comment:
MFT r78108, r78179, r78344, r78347, r78350, r78365, r78380, r78425, r78539, r78886, r78893, r78897, r78909, r78964, r79086, r79087, r79091 (catches up thru Mid-January and rev 80k)
Modified paths:
  • /branches/REL1_17/phase3 (modified) (history)
  • /branches/REL1_17/phase3/includes/HTMLForm.php (modified) (history)
  • /branches/REL1_17/phase3/includes/MessageCache.php (modified) (history)
  • /branches/REL1_17/phase3/includes/media/Bitmap_ClientOnly.php (modified) (history)
  • /branches/REL1_17/phase3/includes/revisiondelete/RevisionDelete.php (modified) (history)
  • /branches/REL1_17/phase3/resources/mediawiki/mediawiki.js (modified) (history)
  • /branches/REL1_17/phase3/skins/chick/main.css (modified) (history)
  • /branches/REL1_17/phase3/skins/common/shared.css (modified) (history)
  • /branches/REL1_17/phase3/skins/common/wikibits.js (modified) (history)
  • /branches/REL1_17/phase3/skins/modern/main.css (modified) (history)
  • /branches/REL1_17/phase3/skins/monobook/main.css (modified) (history)
  • /branches/REL1_17/phase3/skins/vector/screen.css (modified) (history)

Diff [purge]

Index: branches/REL1_17/phase3/skins/chick/main.css
@@ -47,6 +47,7 @@
4848 background: none;
4949 font-weight: normal;
5050 margin: 0;
 51+ overflow: hidden;
5152 padding-top: 0.5em;
5253 padding-bottom: 0.17em;
5354 border-bottom: 1px solid #aaaaaa;
@@ -279,7 +280,6 @@
280281 /* thumbnails */
281282 div.thumb {
282283 margin-bottom: 0.5em;
283 - border-style: solid; border-color: white;
284284 width: auto;
285285 }
286286 div.thumbinner {
@@ -312,13 +312,12 @@
313313 div.tright {
314314 clear: right;
315315 float: right;
316 - border-width: 0.5em 0 0.8em 1.4em;
 316+ margin: 0.5em 0 1.3em 1.4em;
317317 }
318318 div.tleft {
319319 float: left;
320320 clear: left;
321 - margin-right:0.5em;
322 - border-width: 0.5em 1.4em 0.8em 0;
 321+ margin: 0.5em 1.4em 1.3em 0;
323322 }
324323 img.thumbborder {
325324 border: 1px solid #dddddd;
Index: branches/REL1_17/phase3/skins/monobook/main.css
@@ -117,6 +117,7 @@
118118 background: none;
119119 font-weight: normal;
120120 margin: 0;
 121+ overflow: hidden;
121122 padding-top: .5em;
122123 padding-bottom: .17em;
123124 border-bottom: 1px solid #aaa;
@@ -404,8 +405,6 @@
405406 /* thumbnails */
406407 div.thumb {
407408 margin-bottom: .5em;
408 - border-style: solid;
409 - border-color: white;
410409 width: auto;
411410 }
412411 div.thumbinner {
@@ -439,13 +438,12 @@
440439 div.tright {
441440 clear: right;
442441 float: right;
443 - border-width: .5em 0 .8em 1.4em;
 442+ margin: .5em 0 1.3em 1.4em;
444443 }
445444 div.tleft {
446445 float: left;
447446 clear: left;
448 - margin-right: .5em;
449 - border-width: .5em 1.4em .8em 0;
 447+ margin: .5em 1.4em 1.3em 0;
450448 }
451449 img.thumbborder {
452450 border: 1px solid #dddddd;
Index: branches/REL1_17/phase3/skins/modern/main.css
@@ -333,6 +333,10 @@
334334 border-bottom: solid 1px #003366;
335335 }
336336
 337+h1, h2, h3, h4, h5, h6 {
 338+ overflow: hidden;
 339+}
 340+
337341 #preftoc {
338342 width: 100%;
339343 margin: 0 0 0 0;
@@ -487,8 +491,6 @@
488492 /* thumbnails */
489493 div.thumb {
490494 margin-bottom: .5em;
491 - border-style: solid;
492 - border-color: white;
493495 width: auto;
494496 }
495497 div.thumbinner {
@@ -522,13 +524,12 @@
523525 div.tright {
524526 clear: right;
525527 float: right;
526 - border-width: .5em 0 .8em 1.4em;
 528+ margin: .5em 0 .8em 1.4em;
527529 }
528530 div.tleft {
529531 float: left;
530532 clear: left;
531 - margin-right: .5em;
532 - border-width: .5em 1.4em .8em 0;
 533+ margin: .5em 1.4em .8em 0;
533534 }
534535 img.thumbborder {
535536 border: 1px solid #dddddd;
Index: branches/REL1_17/phase3/skins/common/shared.css
@@ -9,6 +9,10 @@
1010 .mw-plusminus-neg { color: #8b0000; } /* dark red */
1111 .mw-plusminus-null { color: #aaa; } /* gray */
1212
 13+/* Links to redirects appear italicized on [[Special:AllPages]],
 14+ and in category listings */
 15+.allpagesredirect, .redirect-in-category { font-style: italic; }
 16+
1317 /* Comment and username portions of RC entries */
1418 span.comment {
1519 font-style: italic;
Property changes on: branches/REL1_17/phase3/skins/common/shared.css
___________________________________________________________________
Modified: svn:mergeinfo
1620 Merged /trunk/phase3/skins/common/shared.css:r78108,78179,78344,78347,78350,78365,78380,78425,78539,78886,78893,78897,78909,78964,79086-79087,79091
Index: branches/REL1_17/phase3/skins/common/wikibits.js
@@ -620,13 +620,13 @@
621621 for ( var i = 0; i < firstRow.cells.length; i++ ) {
622622 var cell = firstRow.cells[i];
623623 if ( (' ' + cell.className + ' ').indexOf(' unsortable ') == -1 ) {
624 - cell.innerHTML += '<a href="#" class="sortheader" '
 624+ $(cell).append ( '<a href="#" class="sortheader" '
625625 + 'onclick="ts_resortTable(this);return false;">'
626626 + '<span class="sortarrow">'
627627 + '<img src="'
628628 + ts_image_path
629629 + ts_image_none
630 - + '" alt="&darr;"/></span></a>';
 630+ + '" alt="&darr;"/></span></a>');
631631 }
632632 }
633633 if ( ts_alternate_row_colors ) {
@@ -1041,8 +1041,9 @@
10421042
10431043 updateTooltipAccessKeys( null );
10441044 setupCheckboxShiftClick();
1045 - sortables_init();
10461045
 1046+ jQuery( document ).ready( sortables_init );
 1047+
10471048 // Run any added-on functions
10481049 for ( var i = 0; i < onloadFuncts.length; i++ ) {
10491050 onloadFuncts[i]();
Index: branches/REL1_17/phase3/skins/vector/screen.css
@@ -710,6 +710,7 @@
711711 background: none;
712712 font-weight: normal;
713713 margin: 0;
 714+ overflow: hidden;
714715 padding-top: .5em;
715716 padding-bottom: .17em;
716717 border-bottom: 1px solid #aaa;
@@ -903,8 +904,6 @@
904905 /* Thumbnails */
905906 div.thumb {
906907 margin-bottom: .5em;
907 - border-style: solid;
908 - border-color: white;
909908 width: auto;
910909 background-color: transparent;
911910 }
@@ -940,14 +939,13 @@
941940 div.tright {
942941 clear: right;
943942 float: right;
944 - border-width: .5em 0 .8em 1.4em;
 943+ margin: .5em 0 1.3em 1.4em;
945944 }
946945 /* @noflip */
947946 div.tleft {
948947 float: left;
949948 clear: left;
950 - margin-right: .5em;
951 - border-width: .5em 1.4em .8em 0;
 949+ margin: .5em 1.4em 1.3em 0;
952950 }
953951 img.thumbborder {
954952 border: 1px solid #dddddd;
Property changes on: branches/REL1_17/phase3/skins/vector/screen.css
___________________________________________________________________
Modified: svn:mergeinfo
955953 Merged /trunk/phase3/skins/vector/screen.css:r78108,78179,78344,78347,78350,78365,78380,78425,78539,78886,78893,78897,78909,78964,79086-79087,79091
Index: branches/REL1_17/phase3/includes/MessageCache.php
@@ -609,18 +609,25 @@
610610 return substr( $entry, 1 );
611611 }
612612 }
 613+ }
 614+ } else {
 615+ // XXX: This is not cached in process cache, should it?
 616+ $message = false;
 617+ wfRunHooks('MessagesPreLoad', array( $title, &$message ) );
 618+ if ( $message !== false ) {
 619+ return $message;
 620+ }
613621
614 - # Call message hooks, in case they are defined
615 - wfRunHooks('MessagesPreLoad', array( $title, &$message ) );
616 - if ( $message !== false ) {
617 - return $message;
 622+ /* If message cache is in normal mode, it is guaranteed
 623+ * (except bugs) that there is always entry (or placeholder)
 624+ * in the cache if message exists. Thus we can do minor
 625+ * performance improvement and return false early.
 626+ */
 627+ if ( !$wgAdaptiveMessageCache ) {
 628+ return false;
 629+ }
618630 }
619631
620 - # If there is no cache entry and no placeholder, it doesn't exist
621 - if ( $type !== '!' ) {
622 - return false;
623 - }
624 -
625632 $titleKey = wfMemcKey( 'messages', 'individual', $title );
626633
627634 # Try the individual message cache
Index: branches/REL1_17/phase3/includes/HTMLForm.php
@@ -187,23 +187,26 @@
188188 }
189189
190190 /**
191 - * The here's-one-I-made-earlier option: do the submission if
192 - * posted, or display the form with or without funky valiation
193 - * errors
194 - * @return Bool or Status whether submission was successful.
 191+ * Prepare form for submission
195192 */
196 - function show() {
 193+ function prepareForm() {
197194 # Check if we have the info we need
198195 if ( ! $this->mTitle ) {
199196 throw new MWException( "You must call setTitle() on an HTMLForm" );
200197 }
201198
 199+ // FIXME shouldn't this be closer to displayForm() ?
202200 self::addJS();
203201
204202 # Load data from the request.
205203 $this->loadData();
 204+ }
206205
207 - # Try a submission
 206+ /**
 207+ * Try submitting, with edit token check first
 208+ * @return Status|boolean
 209+ */
 210+ function tryAuthorizedSubmit() {
208211 global $wgUser, $wgRequest;
209212 $editToken = $wgRequest->getVal( 'wpEditToken' );
210213
@@ -211,12 +214,23 @@
212215 if ( $this->getMethod() != 'post' || $wgUser->matchEditToken( $editToken ) ) {
213216 $result = $this->trySubmit();
214217 }
 218+ return $result;
 219+ }
215220
 221+ /**
 222+ * The here's-one-I-made-earlier option: do the submission if
 223+ * posted, or display the form with or without funky valiation
 224+ * errors
 225+ * @return Bool or Status whether submission was successful.
 226+ */
 227+ function show() {
 228+ $this->prepareForm();
 229+
 230+ $result = $this->tryAuthorizedSubmit();
216231 if ( $result === true || ( $result instanceof Status && $result->isGood() ) ){
217232 return $result;
218233 }
219234
220 - # Display form.
221235 $this->displayForm( $result );
222236 return false;
223237 }
@@ -541,7 +555,6 @@
542556 $this->mSubmitTooltip = $name;
543557 }
544558
545 -
546559 /**
547560 * Set the id for the submit button.
548561 * @param $t String. FIXME: Integrity is *not* validated
Property changes on: branches/REL1_17/phase3/includes/HTMLForm.php
___________________________________________________________________
Modified: svn:mergeinfo
549562 Merged /trunk/phase3/includes/HTMLForm.php:r78108,78179,78344,78347,78350,78365,78380,78425,78539,78886,78893,78897,78909,78964,79086-79087,79091
Index: branches/REL1_17/phase3/includes/revisiondelete/RevisionDelete.php
@@ -220,7 +220,7 @@
221221 */
222222 class RevDel_ArchiveItem extends RevDel_RevisionItem {
223223 public function __construct( $list, $row ) {
224 - parent::__construct( $list, $row );
 224+ RevDel_Item::__construct( $list, $row );
225225 $this->revision = Revision::newFromArchiveRow( $row,
226226 array( 'page' => $this->list->title->getArticleId() ) );
227227 }
@@ -526,7 +526,7 @@
527527 */
528528 class RevDel_ArchivedFileItem extends RevDel_FileItem {
529529 public function __construct( $list, $row ) {
530 - parent::__construct( $list, $row );
 530+ RevDel_Item::__construct( $list, $row );
531531 $this->file = ArchivedFile::newFromRow( $row );
532532 }
533533
Index: branches/REL1_17/phase3/includes/media/Bitmap_ClientOnly.php
@@ -16,7 +16,7 @@
1717 */
1818 class BitmapHandler_ClientOnly extends BitmapHandler {
1919 function normaliseParams( $image, &$params ) {
20 - return parent::normaliseParams( $image, $params );
 20+ return ImageHandler::normaliseParams( $image, $params );
2121 }
2222
2323 function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) {
Index: branches/REL1_17/phase3/resources/mediawiki/mediawiki.js
@@ -226,6 +226,83 @@
227227 */
228228 function User() {
229229 this.options = new Map();
 230+
 231+ /* Public Methods */
 232+
 233+ /*
 234+ * Generates a random user session ID (32 alpha-numeric characters).
 235+ *
 236+ * This information would potentially be stored in a cookie to identify a user during a
 237+ * session or series of sessions. It's uniqueness should not be depended on.
 238+ *
 239+ * @return string random set of 32 alpha-numeric characters
 240+ */
 241+ function generateId() {
 242+ var id = '';
 243+ var seed = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXTZabcdefghiklmnopqrstuvwxyz';
 244+ for ( var i = 0, r; i < 32; i++ ) {
 245+ r = Math.floor( Math.random() * seed.length );
 246+ id += seed.substring( r, r + 1 );
 247+ }
 248+ return id;
 249+ }
 250+
 251+ /*
 252+ * Gets the current user's name.
 253+ *
 254+ * @return mixed user name string or null if users is anonymous
 255+ */
 256+ this.name = function() {
 257+ return mediaWiki.config.get( 'wgUserName' );
 258+ };
 259+
 260+ /*
 261+ * Gets a random session ID automatically generated and kept in a cookie.
 262+ *
 263+ * This ID is ephemeral for everyone, staying in their browser only until they close
 264+ * their browser.
 265+ *
 266+ * Do not use this method before the first call to mediaWiki.loader.go(), it depends on
 267+ * jquery.cookie, which is added to the first pay-load just after mediaWiki is defined, but
 268+ * won't be loaded until the first call to go().
 269+ *
 270+ * @return string user name or random session ID
 271+ */
 272+ this.sessionId = function () {
 273+ var sessionId = $.cookie( 'mediaWiki.user.sessionId' );
 274+ if ( typeof sessionId == 'undefined' || sessionId == null ) {
 275+ sessionId = generateId();
 276+ $.cookie( 'mediaWiki.user.sessionId', sessionId, { 'expires': null, 'path': '/' } );
 277+ }
 278+ return sessionId;
 279+ };
 280+
 281+ /*
 282+ * Gets the current user's name or a random ID automatically generated and kept in a cookie.
 283+ *
 284+ * This ID is persistent for anonymous users, staying in their browser up to 1 year. The
 285+ * expiration time is reset each time the ID is queried, so in most cases this ID will
 286+ * persist until the browser's cookies are cleared or the user doesn't visit for 1 year.
 287+ *
 288+ * Do not use this method before the first call to mediaWiki.loader.go(), it depends on
 289+ * jquery.cookie, which is added to the first pay-load just after mediaWiki is defined, but
 290+ * won't be loaded until the first call to go().
 291+ *
 292+ * @return string user name or random session ID
 293+ */
 294+ this.id = function() {
 295+ var name = that.name();
 296+ if ( name ) {
 297+ return name;
 298+ }
 299+ var id = $.cookie( 'mediaWiki.user.id' );
 300+ if ( typeof id == 'undefined' || id == null ) {
 301+ id = generateId();
 302+ }
 303+ // Set cookie if not set, or renew it if already set
 304+ $.cookie( 'mediaWiki.user.id', id, { 'expires': 365, 'path': '/' } );
 305+ return id;
 306+ };
230307 }
231308
232309 /* Public Members */
@@ -1022,6 +1099,9 @@
10231100 delete startUp;
10241101 }
10251102
 1103+// Add jQuery Cookie to initial payload (used in mediaWiki.user)
 1104+mediaWiki.loader.load( 'jquery.cookie' );
 1105+
10261106 // Alias $j to jQuery for backwards compatibility
10271107 window.$j = jQuery;
10281108 window.mw = mediaWiki;
Property changes on: branches/REL1_17/phase3/resources/mediawiki/mediawiki.js
___________________________________________________________________
Modified: svn:mergeinfo
10291109 Merged /trunk/phase3/resources/mediawiki/mediawiki.js:r78108,78179,78344,78347,78350,78365,78380,78425,78539,78886,78893,78897,78909,78964,79086-79087,79091
Property changes on: branches/REL1_17/phase3
___________________________________________________________________
Modified: svn:mergeinfo
10301110 Merged /trunk/phase3:r78108,78179,78344,78347,78350,78365,78380,78425,78539,78886,78893,78897,78909,78964,79086-79087,79091

Follow-up revisions

RevisionCommit summaryAuthorDate
r85141Fix braces from r85080reedy18:14, 1 April 2011
r85343Fix for r85080: re-merge r78344, r78893, r78897, r78909demon17:13, 4 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78108Changes by Jason Gigliokim02:19, 9 December 2010
r78179Fix regression in r70657. Misplaced else condition was causing cache misses...nikerabbit13:18, 10 December 2010
r78344Fixed issue in r78104 where jquery.cookie wouldn't load properly. Changed the...tparscal23:51, 13 December 2010
r78347Fixes r78104 much better than r78344 did. By adding jquery.cookie to the load...tparscal00:02, 14 December 2010
r78350Added comment about why you should not be depending on mediaWiki.user.session...tparscal00:08, 14 December 2010
r78365Followup r78104: set the cookie unconditionally, so its expiry is renewed eve...catrope11:09, 14 December 2010
r78380Fix up few remaining fixme things from r69910reedy15:44, 14 December 2010
r78425refactored HTMLForm show() into three methods, in case you want to process a ...neilk01:17, 15 December 2010
r78539Split the functionality of mediaWiki.user.sessionId into id and sessionId - t...tparscal18:32, 17 December 2010
r78886Followup r78539: don't renew a cookie that expires at the end of the session ...catrope14:07, 23 December 2010
r78893using .append() instead of innerHTML+=. The latter replaces the html and caus...krinkle16:36, 23 December 2010
r78897didn't mean to commit thiskrinkle17:50, 23 December 2010
r78909Fix r78897: remove dependency on makeCollapsible as wellcatrope18:38, 23 December 2010
r78964Adding default styling for redirects...krinkle15:07, 24 December 2010
r79086Room around thumbs should be margin, not a transparent border....krinkle20:45, 27 December 2010
r79087Adding overflow:hidden to heading elements in Vector....krinkle20:53, 27 December 2010
r79091* Follow-up r79087. Applying to other skins that have similar article styling...krinkle23:23, 27 December 2010

Comments

#Comment by Solitarius (talk | contribs)   18:11, 1 April 2011

This patch cause the following error message: Parse error: syntax error, unexpected T_ELSE in /var/www/w/includes/MessageCache.php on line 613.

Also check r78179.

#Comment by Reedy (talk | contribs)   18:16, 1 April 2011

Fixed in r85141, thanks

#Comment by Nikerabbit (talk | contribs)   07:59, 2 April 2011

wgAdaptiveMessageCache is not supposed to in REL1_17 at all.

#Comment by 😂 (talk | contribs)   16:22, 2 April 2011

Grrrr, merge conflicts confused me. Will sort that out.

#Comment by Catrope (talk | contribs)   21:08, 2 April 2011

Where are the changes to Resources.php ? Where did the revdel and the Bitmap_ClientOnly.php changes come from?

#Comment by 😂 (talk | contribs)   21:10, 2 April 2011

RevDel and Bitmap changes came from r78380.

#Comment by Catrope (talk | contribs)   21:10, 2 April 2011

And where are the GoogleNewsSitemap changes?

#Comment by 😂 (talk | contribs)   21:10, 2 April 2011

That was a mistake, shouldn't have mentioned that rev here.

#Comment by 😂 (talk | contribs)   21:13, 2 April 2011

I really don't know where Resources.php disappeared to. I got a *lot* of conflicts in it, and after merging it only showed prop changes and no net text changes (I thought maybe they'd been merged but weren't tagged as such).

#Comment by 😂 (talk | contribs)   17:14, 4 April 2011

Grabbed the ones mentioned in this rev, see r85343

#Comment by 😂 (talk | contribs)   17:15, 4 April 2011

All the concerns here have been addressed -> new.

Status & tagging log