r78893 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78892‎ | r78893 | r78894 >
Date:16:36, 23 December 2010
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
using .append() instead of innerHTML+=. The latter replaces the html and causes all bound event handlers to be list
Modified paths:
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/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: trunk/phase3/resources/Resources.php
@@ -78,6 +78,10 @@
7979 'jquery.localize' => array(
8080 'scripts' => 'resources/jquery/jquery.localize.js'
8181 ),
 82+ 'jquery.makeCollapsible' => array(
 83+ 'scripts' => 'resources/jquery/jquery.makeCollapsible.js',
 84+ 'styles' => 'resources/jquery/jquery.makeCollapsible.css',
 85+ ),
8286 'jquery.suggestions' => array(
8387 'scripts' => 'resources/jquery/jquery.suggestions.js',
8488 'styles' => 'resources/jquery/jquery.suggestions.css',
@@ -345,7 +349,7 @@
346350 ),
347351 'mediawiki.util' => array(
348352 'scripts' => 'resources/mediawiki.util/mediawiki.util.js',
349 - 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client', 'jquery.placeholder' ),
 353+ 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client', 'jquery.placeholder', 'jquery.makeCollapsible' ),
350354 'debugScripts' => 'resources/mediawiki.util/mediawiki.util.test.js',
351355 ),
352356 'mediawiki.action.history' => array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r78897didn't mean to commit thiskrinkle17:50, 23 December 2010
r825331.17wmf1: MFT r78893, r78897, r78909, r82404, r82408, r82409, r82453, r82456,...catrope20:13, 20 February 2011
r85080MFT r78108, r78179, r78344, r78347, r78350, r78365, r78380, r78425, r78539, r...demon19:09, 31 March 2011
r85343Fix for r85080: re-merge r78344, r78893, r78897, r78909demon17:13, 4 April 2011

Comments

#Comment by Catrope (talk | contribs)   17:46, 23 December 2010
+	'jquery.makeCollapsible' => array(
+		'scripts' => 'resources/jquery/jquery.makeCollapsible.js',
+		'styles' => 'resources/jquery/jquery.makeCollapsible.css',
+	),

You forgot to add these files, which causes exceptions in load.php

#Comment by Krinkle (talk | contribs)   17:51, 23 December 2010

Fixed in r78897

#Comment by Krinkle (talk | contribs)   22:08, 19 February 2011

en.wikipedia's collapsibletable script has problems because of the binding with table cells. Perhaps we should backport this to 1.17wmf1. Seems a simple fix ?


Tagging 1.17wmf1

Status & tagging log