r68598 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68597‎ | r68598 | r68599 >
Date:05:22, 26 June 2010
Author:dantman
Status:reverted (Comments)
Tags:
Comment:
Clean up the mess of horrid javascript in wikibits before I fix a bug
Modified paths:
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/wikibits.js
——@@ -5,18 +5,18 @@
66 !/khtml|spoofer|netscape\/7\.0/.test(clientPC);
77 var webkit_match = clientPC.match(/applewebkit\/(\d+)/);
88 if (webkit_match) {
9 - var is_safari = clientPC.indexOf('applewebkit') != -1 &&
10 - clientPC.indexOf('spoofer') == -1;
11 - var is_safari_win = is_safari && clientPC.indexOf('windows') != -1;
 9+ var is_safari = clientPC.indexOf('applewebkit') !== -1 &&
 10+ clientPC.indexOf('spoofer') === -1;
 11+ var is_safari_win = is_safari && clientPC.indexOf('windows') !== -1;
1212 var webkit_version = parseInt(webkit_match[1]);
1313 }
1414 // For accesskeys; note that FF3+ is included here!
1515 var is_ff2 = /firefox\/[2-9]|minefield\/3/.test( clientPC );
1616 var ff2_bugs = /firefox\/2/.test( clientPC );
1717 // These aren't used here, but some custom scripts rely on them
18 -var is_ff2_win = is_ff2 && clientPC.indexOf('windows') != -1;
19 -var is_ff2_x11 = is_ff2 && clientPC.indexOf('x11') != -1;
20 -if (clientPC.indexOf('opera') != -1) {
 18+var is_ff2_win = is_ff2 && clientPC.indexOf('windows') !== -1;
 19+var is_ff2_x11 = is_ff2 && clientPC.indexOf('x11') !== -1;
 20+if (clientPC.indexOf('opera') !== -1) {
2121 var is_opera = true;
2222 var is_opera_preseven = window.opera && !document.childNodes;
2323 var is_opera_seven = window.opera && document.childNodes;
——@@ -28,11 +28,7 @@
2929 // As recommended by <http://msdn.microsoft.com/en-us/library/ms537509.aspx>,
3030 // avoiding false positives from moronic extensions that append to the IE UA
3131 // string (bug 23171)
32 -var ie6_bugs = false;
33 -if ( /MSIE ([0-9]{1,}[\.0-9]{0,})/.exec( clientPC ) != null
34 -&& parseFloat( RegExp.$1 ) <= 6.0 ) {
35 - ie6_bugs = true;
36 -}
 32+var ie6_bugs = /MSIE ([0-9]{1,}[\.0-9]{0,})/.exec( clientPC ) && parseFloat( RegExp.$1 ) <= 6.0 );
3733
3834 // Global external objects used by this script.
3935 /*extern ta, stylepath, skin */
——@@ -68,7 +64,7 @@
6965 var loadedScripts = {}; // included-scripts tracker
7066 function importScriptURI( url ) {
7167 if ( loadedScripts[url] ) {
72 - return null;
 68+ return;
7369 }
7470 loadedScripts[url] = true;
7571 var s = document.createElement( 'script' );
——@@ -108,7 +104,7 @@
109105 }
110106
111107 // Special stylesheet links for Monobook only (see bug 14717)
112 -if ( typeof stylepath != 'undefined' && skin == 'monobook' ) {
 108+if ( typeof stylepath !== 'undefined' && skin === 'monobook' ) {
113109 if ( opera6_bugs ) {
114110 importStylesheetURI( stylepath + '/' + skin + '/Opera6Fixes.css' );
115111 } else if ( opera7_bugs ) {
——@@ -158,7 +154,7 @@
159155 linkHolder.appendChild( outerSpan );
160156
161157 var cookiePos = document.cookie.indexOf( "hidetoc=" );
162 - if ( cookiePos > -1 && document.cookie.charAt( cookiePos + 8 ) == 1 ) {
 158+ if ( cookiePos > -1 && document.cookie.charAt( cookiePos + 8 ) === 1 ) {
163159 toggleToc();
164160 }
165161 }
——@@ -175,7 +171,7 @@
176172
177173 function killEvt( evt ) {
178174 evt = evt || window.event || window.Event; // W3C, IE, Netscape
179 - if ( typeof ( evt.preventDefault ) != 'undefined' ) {
 175+ if ( typeof ( evt.preventDefault ) !== 'undefined' ) {
180176 evt.preventDefault(); // Don't follow the link
181177 evt.stopPropagation();
182178 } else {
——@@ -189,7 +185,7 @@
190186 var toc = document.getElementById('toc').getElementsByTagName('ul')[0];
191187 var toggleLink = document.getElementById( 'togglelink' );
192188
193 - if ( toc && toggleLink && toc.style.display == 'none' ) {
 189+ if ( toc && toggleLink && toc.style.display === 'none' ) {
194190 changeText( toggleLink, tocHideText );
195191 toc.style.display = 'block';
196192 document.cookie = "hidetoc=0";
——@@ -235,8 +231,8 @@
236232 } else if ( !is_safari_win && is_safari && webkit_version > 526 ) {
237233 tooltipAccessKeyPrefix = 'ctrl-alt-';
238234 } else if ( !is_safari_win && ( is_safari
239 - || clientPC.indexOf('mac') != -1
240 - || clientPC.indexOf('konqueror') != -1 ) ) {
 235+ || clientPC.indexOf('mac') !== -1
 236+ || clientPC.indexOf('konqueror') !== -1 ) ) {
241237 tooltipAccessKeyPrefix = 'ctrl-';
242238 } else if ( is_ff2 ) {
243239 tooltipAccessKeyPrefix = 'alt-shift-';
——@@ -314,11 +310,11 @@
315311 function addPortletLink( portlet, href, text, id, tooltip, accesskey, nextnode ) {
316312 var root = document.getElementById( portlet );
317313 if ( !root ) {
318 - return null;
 314+ return;
319315 }
320316 var node = root.getElementsByTagName( 'ul' )[0];
321317 if ( !node ) {
322 - return null;
 318+ return;
323319 }
324320
325321 // unhide portlet if it was hidden before
——@@ -348,7 +344,7 @@
349345 updateTooltipAccessKeys( new Array( link ) );
350346 }
351347
352 - if ( nextnode && nextnode.parentNode == node ) {
 348+ if ( nextnode && nextnode.parentNode === node ) {
353349 node.insertBefore( item, nextnode );
354350 } else {
355351 node.appendChild( item ); // IE compatibility (?)
——@@ -358,10 +354,10 @@
359355 }
360356
361357 function getInnerText( el ) {
362 - if ( typeof el == 'string' ) {
 358+ if ( typeof el === 'string' ) {
363359 return el;
364360 }
365 - if ( typeof el == 'undefined' ) {
 361+ if ( typeof el === 'undefined' ) {
366362 return el;
367363 }
368364 if ( el.textContent ) {
——@@ -397,7 +393,7 @@
398394
399395 function setupCheckboxShiftClick() {
400396 checkboxes = [];
401 - lastCheckbox = null;
 397+ lastCheckbox = undefined;
402398 var inputs = document.getElementsByTagName( 'input' );
403399 addCheckboxClickHandlers( inputs );
404400 }
——@@ -414,7 +410,7 @@
415411
416412 for ( var i = start; i < finish; i++ ) {
417413 var cb = inputs[i];
418 - if ( !cb.type || cb.type.toLowerCase() != 'checkbox' ) {
 414+ if ( !cb.type || cb.type.toLowerCase() !== 'checkbox' ) {
419415 continue;
420416 }
421417 var end = checkboxes.length;
——@@ -431,10 +427,10 @@
432428 }
433429
434430 function checkboxClickHandler( e ) {
435 - if ( typeof e == 'undefined' ) {
 431+ if ( typeof e === 'undefined' ) {
436432 e = window.event;
437433 }
438 - if ( !e.shiftKey || lastCheckbox === null ) {
 434+ if ( !e.shiftKey || lastCheckbox === undefined ) {
439435 lastCheckbox = this.index;
440436 return true;
441437 }
——@@ -449,7 +445,7 @@
450446 }
451447 for ( var i = start; i <= finish; ++i ) {
452448 checkboxes[i].checked = endState;
453 - if( i > start && typeof checkboxes[i].onchange == 'function' ) {
 449+ if( i > start && typeof checkboxes[i].onchange === 'function' ) {
454450 checkboxes[i].onchange(); // fire triggers
455451 }
456452 }
——@@ -466,22 +462,22 @@
467463 */
468464 function getElementsByClassName( oElm, strTagName, oClassNames ) {
469465 var arrReturnElements = new Array();
470 - if ( typeof( oElm.getElementsByClassName ) == 'function' ) {
 466+ if ( typeof( oElm.getElementsByClassName ) === 'function' ) {
471467 /* Use a native implementation where possible FF3, Saf3.2, Opera 9.5 */
472468 var arrNativeReturn = oElm.getElementsByClassName( oClassNames );
473 - if ( strTagName == '*' ) {
 469+ if ( strTagName === '*' ) {
474470 return arrNativeReturn;
475471 }
476472 for ( var h = 0; h < arrNativeReturn.length; h++ ) {
477 - if( arrNativeReturn[h].tagName.toLowerCase() == strTagName.toLowerCase() ) {
 473+ if( arrNativeReturn[h].tagName.toLowerCase() === strTagName.toLowerCase() ) {
478474 arrReturnElements[arrReturnElements.length] = arrNativeReturn[h];
479475 }
480476 }
481477 return arrReturnElements;
482478 }
483 - var arrElements = ( strTagName == '*' && oElm.all ) ? oElm.all : oElm.getElementsByTagName( strTagName );
 479+ var arrElements = ( strTagName === '*' && oElm.all ) ? oElm.all : oElm.getElementsByTagName( strTagName );
484480 var arrRegExpClassNames = new Array();
485 - if( typeof oClassNames == 'object' ) {
 481+ if( typeof oClassNames === 'object' ) {
486482 for( var i = 0; i < oClassNames.length; i++ ) {
487483 arrRegExpClassNames[arrRegExpClassNames.length] =
488484 new RegExp("(^|\\s)" + oClassNames[i].replace(/\-/g, "\\-") + "(\\s|$)");
——@@ -518,7 +514,7 @@
519515 return;
520516 }
521517 }
522 - if ( window.location.hash == '' ) {
 518+ if ( window.location.hash === '' ) {
523519 window.location.hash = fragment;
524520
525521 // Mozilla needs to wait until after load, otherwise the window doesn't
——@@ -529,7 +525,7 @@
530526 // well.
531527 if ( is_gecko ) {
532528 addOnloadHook(function() {
533 - if ( window.location.hash == fragment ) {
 529+ if ( window.location.hash === fragment ) {
534530 window.location.hash = fragment;
535531 }
536532 });
——@@ -552,10 +548,10 @@
553549 var ts_image_up = 'sort_up.gif';
554550 var ts_image_down = 'sort_down.gif';
555551 var ts_image_none = 'sort_none.gif';
556 -var ts_europeandate = wgContentLanguage != 'en'; // The non-American-inclined can change to "true"
 552+var ts_europeandate = wgContentLanguage !== 'en'; // The non-American-inclined can change to "true"
557553 var ts_alternate_row_colors = false;
558 -var ts_number_transform_table = null;
559 -var ts_number_regex = null;
 554+var ts_number_transform_table;
 555+var ts_number_regex;
560556
561557 function sortables_init() {
562558 var idnum = 0;
——@@ -586,7 +582,7 @@
587583 // We have a first row: assume it's the header, and make its contents clickable links
588584 for ( var i = 0; i < firstRow.cells.length; i++ ) {
589585 var cell = firstRow.cells[i];
590 - if ( (' ' + cell.className + ' ').indexOf(' unsortable ') == -1 ) {
 586+ if ( (' ' + cell.className + ' ').indexOf(' unsortable ') === -1 ) {
591587 cell.innerHTML += '<a href="#" class="sortheader" '
592588 + 'onclick="ts_resortTable(this);return false;">'
593589 + '<span class="sortarrow">'
——@@ -614,7 +610,7 @@
615611 var column = td.cellIndex;
616612
617613 var table = tr.parentNode;
618 - while ( table && !( table.tagName && table.tagName.toLowerCase() == 'table' ) ) {
 614+ while ( table && !( table.tagName && table.tagName.toLowerCase() === 'table' ) ) {
619615 table = table.parentNode;
620616 }
621617 if ( !table ) {
——@@ -626,7 +622,7 @@
627623 }
628624
629625 // Generate the number transform table if it's not done already
630 - if ( ts_number_transform_table === null ) {
 626+ if ( ts_number_transform_table === undefined ) {
631627 ts_initTransformTable();
632628 }
633629
——@@ -634,7 +630,7 @@
635631 // Skip the first row if that's where the headings are
636632 var rowStart = ( table.tHead && table.tHead.rows.length > 0 ? 0 : 1 );
637633 var bodyRows = 0;
638 - if (rowStart == 0 && table.tBodies) {
 634+ if (rowStart === 0 && table.tBodies) {
639635 for (var i=0; i < table.tBodies.length; i++ ) {
640636 bodyRows += table.tBodies[i].rows.length;
641637 }
——@@ -647,7 +643,7 @@
648644 if ( table.rows[i].cells.length > column ) {
649645 itm = ts_getInnerText(table.rows[i].cells[column]);
650646 itm = itm.replace(/^[\s\xa0]+/, '').replace(/[\s\xa0]+$/, '');
651 - if ( itm != '' ) {
 647+ if ( itm !== '' ) {
652648 break;
653649 }
654650 }
——@@ -669,7 +665,7 @@
670666 preprocessor = ts_parseFloat;
671667 }
672668
673 - var reverse = ( span.getAttribute( 'sortdir' ) == 'down' );
 669+ var reverse = ( span.getAttribute( 'sortdir' ) === 'down' );
674670
675671 var newRows = new Array();
676672 var staticRows = new Array();
——@@ -709,13 +705,13 @@
710706 // We appendChild rows that already exist to the tbody, so it moves them rather than creating new ones
711707 // don't do sortbottom rows
712708 for ( var i = 0; i < newRows.length; i++ ) {
713 - if ( ( ' ' + newRows[i][0].className + ' ').indexOf(' sortbottom ') == -1 ) {
 709+ if ( ( ' ' + newRows[i][0].className + ' ').indexOf(' sortbottom ') === -1 ) {
714710 table.tBodies[0].appendChild( newRows[i][0] );
715711 }
716712 }
717713 // do sortbottom rows only
718714 for ( var i = 0; i < newRows.length; i++ ) {
719 - if ( ( ' ' + newRows[i][0].className + ' ').indexOf(' sortbottom ') != -1 ) {
 715+ if ( ( ' ' + newRows[i][0].className + ' ').indexOf(' sortbottom ') !== -1 ) {
720716 table.tBodies[0].appendChild( newRows[i][0] );
721717 }
722718 }
——@@ -733,8 +729,8 @@
734730 }
735731
736732 function ts_initTransformTable() {
737 - if ( typeof wgSeparatorTransformTable == 'undefined'
738 - || ( wgSeparatorTransformTable[0] == '' && wgDigitTransformTable[2] == '' ) )
 733+ if ( typeof wgSeparatorTransformTable === 'undefined'
 734+ || ( wgSeparatorTransformTable[0] === '' && wgDigitTransformTable[2] === '' ) )
739735 {
740736 var digitClass = "[0-9,.]";
741737 ts_number_transform_table = false;
——@@ -791,7 +787,7 @@
792788
793789 function ts_dateToSortKey( date ) {
794790 // y2k notes: two digit years less than 50 are treated as 20XX, greater than 50 are treated as 19XX
795 - if ( date.length == 11 ) {
 791+ if ( date.length === 11 ) {
796792 switch ( date.substr( 3, 3 ).toLowerCase() ) {
797793 case 'jan':
798794 var month = '01';
——@@ -832,20 +828,20 @@
833829 // default: var month = '00';
834830 }
835831 return date.substr( 7, 4 ) + month + date.substr( 0, 2 );
836 - } else if ( date.length == 10 ) {
837 - if ( ts_europeandate == false ) {
 832+ } else if ( date.length === 10 ) {
 833+ if ( ts_europeandate === false ) {
838834 return date.substr( 6, 4 ) + date.substr( 0, 2 ) + date.substr( 3, 2 );
839835 } else {
840836 return date.substr( 6, 4 ) + date.substr( 3, 2 ) + date.substr( 0, 2 );
841837 }
842 - } else if ( date.length == 8 ) {
 838+ } else if ( date.length === 8 ) {
843839 var yr = date.substr( 6, 2 );
844840 if ( parseInt( yr ) < 50 ) {
845841 yr = '20' + yr;
846842 } else {
847843 yr = '19' + yr;
848844 }
849 - if ( ts_europeandate == true ) {
 845+ if ( ts_europeandate === true ) {
850846 return yr + date.substr( 3, 2 ) + date.substr( 0, 2 );
851847 } else {
852848 return yr + date.substr( 0, 2 ) + date.substr( 3, 2 );
——@@ -858,7 +854,7 @@
859855 if ( !s ) {
860856 return 0;
861857 }
862 - if ( ts_number_transform_table != false ) {
 858+ if ( ts_number_transform_table !== false ) {
863859 var newNum = '', c;
864860
865861 for ( var p = 0; p < s.length; p++ ) {
——@@ -897,11 +893,11 @@
898894 var oldClasses = tableRows[j].className.split(' ');
899895 var newClassName = '';
900896 for ( var k = 0; k < oldClasses.length; k++ ) {
901 - if ( oldClasses[k] != '' && oldClasses[k] != 'even' && oldClasses[k] != 'odd' ) {
 897+ if ( oldClasses[k] !== '' && oldClasses[k] !== 'even' && oldClasses[k] !== 'odd' ) {
902898 newClassName += oldClasses[k] + ' ';
903899 }
904900 }
905 - tableRows[j].className = newClassName + ( j % 2 == 0 ? 'even' : 'odd' );
 901+ tableRows[j].className = newClassName + ( j % 2 === 0 ? 'even' : 'odd' );
906902 }
907903 }
908904 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r68611Revert r68598. Broken....siebrand20:39, 26 June 2010

Comments

#Comment by Nikerabbit (talk | contribs)   13:51, 26 June 2010 
Uncaught SyntaxError: Unexpected token ) http://bits.translatewiki.net/w/skins/common/wikibits.js?292t5:36

Probably this line: 

+var ie6_bugs = /MSIE ([0-9]{1,}[\.0-9]{0,})/.exec( clientPC ) && parseFloat( RegExp.$1 ) <= 6.0 );
#Comment by Simetrical (talk | contribs)   21:54, 1 July 2010

Do not mass-change == to ===. Only change specific instances where you can identify a particular problem that might occur. Otherwise you're going to break stuff. I once spent like two hours tracking down a JavaScript-related bug, and it turned out to be because someone decided they should change == to === everywhere without actually testing every change.

Status & tagging log