r78941 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78940‎ | r78941 | r78942 >
Date:00:55, 24 December 2010
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
Porting tocToggle to ResourceLoader
* Path is now set to sitewide rather than current path so that it doesn't apply to just the current article but to the entire domain (fixes bug 26324)
* To avoid cookievalue being overriden by an older cookie from the legacy code (which had a more specific path and thus overrides it), using a different cookiename now ("mw_hidetoc" instead of "hidetoc")
Modified paths:
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.util/mediawiki.util.js (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/wikibits.js
@@ -136,42 +136,6 @@
137137 }
138138 }
139139
140 -window.showTocToggle = function() {
141 - if ( document.createTextNode ) {
142 - // Uses DOM calls to avoid document.write + XHTML issues
143 -
144 - var linkHolder = document.getElementById( 'toctitle' );
145 - var existingLink = document.getElementById( 'togglelink' );
146 - if ( !linkHolder || existingLink ) {
147 - // Don't add the toggle link twice
148 - return;
149 - }
150 -
151 - var outerSpan = document.createElement( 'span' );
152 - outerSpan.className = 'toctoggle';
153 -
154 - var toggleLink = document.createElement( 'a' );
155 - toggleLink.id = 'togglelink';
156 - toggleLink.className = 'internal';
157 - toggleLink.href = '#';
158 - addClickHandler( toggleLink, function( evt ) { toggleToc(); return killEvt( evt ); } );
159 -
160 - toggleLink.appendChild( document.createTextNode( mediaWiki.msg( 'hidetoc' ) ) );
161 -
162 - outerSpan.appendChild( document.createTextNode( '[' ) );
163 - outerSpan.appendChild( toggleLink );
164 - outerSpan.appendChild( document.createTextNode( ']' ) );
165 -
166 - linkHolder.appendChild( document.createTextNode( ' ' ) );
167 - linkHolder.appendChild( outerSpan );
168 -
169 - var cookiePos = document.cookie.indexOf( "hidetoc=" );
170 - if ( cookiePos > -1 && document.cookie.charAt( cookiePos + 8 ) == 1 ) {
171 - toggleToc();
172 - }
173 - }
174 -};
175 -
176140 window.changeText = function( el, newText ) {
177141 // Safari work around
178142 if ( el.innerText ) {
@@ -192,25 +156,6 @@
193157 return false; // Don't follow the link (IE)
194158 };
195159
196 -window.toggleToc = function() {
197 - var tocmain = document.getElementById( 'toc' );
198 - var toc = document.getElementById('toc').getElementsByTagName('ul')[0];
199 - var toggleLink = document.getElementById( 'togglelink' );
200 -
201 - if ( toc && toggleLink && toc.style.display == 'none' ) {
202 - changeText( toggleLink, mediaWiki.msg( 'hidetoc' ) );
203 - toc.style.display = 'block';
204 - document.cookie = "hidetoc=0";
205 - tocmain.className = 'toc';
206 - } else {
207 - changeText( toggleLink, mediaWiki.msg( 'showtoc' ) );
208 - toc.style.display = 'none';
209 - document.cookie = "hidetoc=1";
210 - tocmain.className = 'toc tochidden';
211 - }
212 - return false;
213 -};
214 -
215160 window.mwEditButtons = [];
216161 window.mwCustomEditButtons = []; // eg to add in MediaWiki:Common.js
217162
@@ -1099,6 +1044,4 @@
11001045
11011046 if ( ie6_bugs ) {
11021047 importScriptURI( stylepath + '/common/IEFixes.js' );
1103 -}
1104 -
1105 -showTocToggle();
 1048+}
\ No newline at end of file
Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
@@ -15,10 +15,10 @@
1616 // Any initialisation after the DOM is ready
1717 $(function () {
1818
19 - // Shortcut
 19+ // Shortcut to client profile return
2020 var profile = $.client.profile();
2121
22 - // Set tooltipAccessKeyPrefix
 22+ /* Set tooltipAccessKeyPrefix */
2323
2424 // Opera on any platform
2525 if ( profile.name == 'opera' ) {
@@ -49,15 +49,15 @@
5050 mw.util.tooltipAccessKeyPrefix = 'alt-shift-';
5151 }
5252
53 - // Enable CheckboxShiftClick
 53+ /* Enable CheckboxShiftClick */
5454 $( 'input[type=checkbox]:not(.noshiftselect)' ).checkboxShiftClick();
5555
56 - // Emulate placeholder if not supported by browser
 56+ /* Emulate placeholder if not supported by browser */
5757 if ( !( 'placeholder' in document.createElement( 'input' ) ) ) {
5858 $( 'input[placeholder]' ).placeholder();
5959 }
6060
61 - // Fill $content var
 61+ /* Fill $content var */
6262 if ( $( '#bodyContent' ).length ) {
6363 mw.util.$content = $( '#bodyContent' );
6464 } else if ( $( '#article' ).length ) {
@@ -65,6 +65,25 @@
6666 } else {
6767 mw.util.$content = $( '#content' );
6868 }
 69+
 70+ /* Table of Contents toggle */
 71+ var $tocContainer = $( '#toc' ),
 72+ $tocTitle = $( '#toctitle' ),
 73+ $tocToggleLink = $( '#togglelink' );
 74+ // Only add it if there is a TOC and there is no toggle added already
 75+ if ( $tocContainer.size() && $tocTitle.size() && !$tocToggleLink.size() ) {
 76+ var hideTocCookie = $.cookie( 'mw_hidetoc' ),
 77+ $tocToggleLink = $( '<a href="#" class="internal" id="togglelink">' ).text( mw.msg( 'hidetoc' ) ).click( function(e){
 78+ e.preventDefault();
 79+ mw.util.toggleToc( $(this) );
 80+ } );
 81+ $tocTitle.append( $tocToggleLink.wrap( '<span class="toctoggle">' ).parent().prepend( '&nbsp;[' ).append( ']&nbsp;' ) );
 82+
 83+ if ( hideTocCookie == '1' ) {
 84+ // Cookie says user want toc hidden
 85+ $tocToggleLink.click();
 86+ }
 87+ }
6988 } );
7089
7190 return true;
@@ -118,6 +137,32 @@
119138 },
120139
121140 /**
 141+ * Hide/show the table of contents element
 142+ *
 143+ * @param text String CSS to be appended
 144+ * @return the CSS stylesheet
 145+ */
 146+ 'toggleToc' : function( $toggleLink ) {
 147+ var $tocList = $( '#toc ul:first' ),
 148+
 149+ if ( $tocList.is( ':hidden' ) ) {
 150+ $tocList.slideDown( 'fast' );
 151+ $toggleLink.text( mw.msg( 'hidetoc' ) );
 152+ $.cookie( 'mw_hidetoc', null, {
 153+ expires: 30,
 154+ path: '/'
 155+ } );
 156+ } else {
 157+ $tocList.slideUp( 'fast' );
 158+ $toggleLink.text( mw.msg( 'showtoc' ) );
 159+ $.cookie( 'mw_hidetoc', '1', {
 160+ expires: 30,
 161+ path: '/'
 162+ } );
 163+ }
 164+ },
 165+
 166+ /**
122167 * Get the full URL to a page name
123168 *
124169 * @param str Page name to link to
Index: trunk/phase3/resources/Resources.php
@@ -350,7 +350,7 @@
351351 ),
352352 'mediawiki.util' => array(
353353 'scripts' => 'resources/mediawiki.util/mediawiki.util.js',
354 - 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client', 'jquery.placeholder', 'jquery.makeCollapsible' ),
 354+ 'dependencies' => array( 'jquery.checkboxShiftClick', 'jquery.client', 'jquery.cookie', 'jquery.placeholder', 'jquery.makeCollapsible' ),
355355 'debugScripts' => 'resources/mediawiki.util/mediawiki.util.test.js',
356356 ),
357357 'mediawiki.action.history' => array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r78959Make sure the TOC is there when toggleToc is called. Although the function ca...krinkle13:44, 24 December 2010
r89349follow up r88502 with a fix to resource loader to restore tochidden...mah16:38, 2 June 2011

Comments

#Comment by Platonides (talk | contribs)   11:55, 24 December 2010
Error: missing variable name
Source file: load.php?debug=false&modules=jquery.checkboxShiftClick%7Cjquery.client%7Cjquery.cookie%7Cjquery.effects.core%7Cjquery.effects.fade%7Cjquery.makeCollapsible%7Cjquery.placeholder%7Cmediawiki.language%7Cmediawiki.legacy.ajax%7Cmediawiki.legacy.wikibits%7Cmediawiki.util%7Cskins.common
Line: 2778
Source: if($tocList.is(':hidden')){ 

I think you missed that not all have a toc. This happens in non-TOC pages, edit pages...

#Comment by Krinkle (talk | contribs)   12:23, 24 December 2010

When does this occur ? When I visit a normal page without a TOC in my local install I dont see any errors.

The following piece cited from this commit is exactly what checks that there is a valid TOC present:

+
+					/* Table of Contents toggle */
+					var $tocContainer = $( '#toc' ),
+						$tocTitle = $( '#toctitle' ),
+						$tocToggleLink = $( '#togglelink' );
+					// Only add it if there is a TOC and there is no toggle added already
+					if ( $tocContainer.size() && $tocTitle.size() && !$tocToggleLink.size() ) {
#Comment by Platonides (talk | contribs)   13:36, 24 December 2010

Visiting a non-TOC page, or even an edit page.

Yes, all three

  • $('#toc').size()
  • $('#toctitle').size()
  • $('#togglelink').size()

are zero.

But it errors inside toggleToc :/

#Comment by Krinkle (talk | contribs)   13:37, 24 December 2010

So you are calling mw.util.toggleToc() yourself ?

#Comment by Platonides (talk | contribs)   13:38, 24 December 2010

I think

var $tocList = $( '#toc ul:first' ),

should en in ; instead of , ?

#Comment by Krinkle (talk | contribs)   03:47, 29 May 2011

Has been fixed in the mean time. Thanks.

#Comment by Fomafix (talk | contribs)   09:13, 31 May 2011

The css class tochidden is lost. It is used to hide the entire TOC box in print mode (Bug 482). .tochidden { display: none } is still in commonPrint.css. See Bug 29043.

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

Fixed by Mark H in r89349

Status & tagging log