MediaWiki r90239 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r90238‎ | r90239 (on ViewVC)‎ | r90240 >
Date:22:27, 16 June 2011
Author:mah
Status:ok (Comments)
Tags:
Comment:
Fix Bug #28344 - “Links doesn't work inside collapsible toggles”
Patch from mybugs.mail

If a link is added inside of a collapsible toggle and a user click on it, the
target page should be opened.
…
The attached patch was tested locally and fixes this bug (clicking outside the
link toogles the collapsed content, and clicking on the link opens the target
page).

PS: It is the same code which was used to fix the problem on Commons.

http://commons.wikimedia.org/w/index.php?diff=45383826&amp;oldid=45265712
Modified paths:

Diff [purge]

Index: trunk/phase3/CREDITS
——@@ -125,6 +125,7 @@
126126 * Mormegil
127127 * MrPete
128128 * MZMcBride
 129+* mybugs.mail
129130 * Nakon
130131 * Nathan Larson
131132 * nephele
Index: trunk/phase3/resources/jquery/jquery.makeCollapsible.js
——@@ -176,6 +176,9 @@
177177 },
178178 // Toggles collapsible and togglelink class
179179 toggleLinkPremade = function( $that, e ) {
 180+ if ( $(e.target).is('a') ) {
 181+ return true;
 182+ }
180183 var $collapsible = $that.eq(0).closest( '.mw-collapsible.mw-made-collapsible' ).toggleClass( 'mw-collapsed' );
181184 e.preventDefault();
182185 e.stopPropagation();

Follow-up revisions

Rev.Commit summaryAuthorDate
r90851Followup r90239: Move return till after the toggle is executed.mah20:00, 26 June 2011

Comments

#Comment by Krinkle (talk | contribs)   01:32, 25 June 2011

I can't check it out right now, but shouldn't this be after : 

 				var	$collapsible = $that.eq(0).closest( '.mw-collapsible.mw-made-collapsible' ).toggleClass( 'mw-collapsed' );

Right now the return is making it return early.

#Comment by Krinkle (talk | contribs)   01:34, 25 June 2011

I get that a link would take the user to a new address, so what would it matter ? Well not every Anchor tag is an external link. It could be linking to a section on the current page or something that is also bound by javascript, in which case the collapse/expand should still happen.

Status & tagging log

Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/90239"