From 15b88fc2807c21c4fbefbb75ba2433d3d150b34a Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 25 May 2017 23:08:24 +0200
Subject: [PATCH] jquery.makeCollapsible: Restore considering empty <a> as part
 of toggle

Before 2d95d36a8e, clicks on links inside toggles with non-empty
targets that are not '#', were ignored ("pass through") since they
are not intended for the toggle.

In 2d95d36a8e, this was simplified to ignoring clicks from all <a>
elements inside toggles. However this ignored too much as links
without 'href' attribute are also sometimes used inside toggles
to look like links and have no href-target, which means clicking
them does nothing and is in fact meant to toggle the element.

Restore previous behaviour and restore previous test + add a new
test for this specific case.

Bug: T166298
Change-Id: Ia3a0648f809f94be0977a83b469fbd184aa72aff
---
 resources/src/jquery/jquery.makeCollapsible.js |  3 ++-
 .../jquery/jquery.makeCollapsible.test.js      | 18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/resources/src/jquery/jquery.makeCollapsible.js b/resources/src/jquery/jquery.makeCollapsible.js
index 9d3df8cbf3..ac4a3926ac 100644
--- a/resources/src/jquery/jquery.makeCollapsible.js
+++ b/resources/src/jquery/jquery.makeCollapsible.js
@@ -152,7 +152,8 @@
 		if ( e ) {
 			if (
 				e.type === 'click' &&
-				e.target.nodeName.toLowerCase() === 'a'
+				e.target.nodeName.toLowerCase() === 'a' &&
+				$( e.target ).attr( 'href' )
 			) {
 				// Don't fire if a link was clicked (for premade togglers)
 				return;
diff --git a/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js b/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js
index 44a2305352..53d29cf93e 100644
--- a/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js
+++ b/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js
@@ -266,7 +266,7 @@
 		var $collapsible = prepareCollapsible(
 				'<div class="mw-collapsible">' +
 					'<div class="mw-collapsible-toggle">' +
-						'Toggle <a href="#">toggle</a> toggle <b>toggle</b>' +
+						'Toggle <a href="#top">toggle</a> toggle <b>toggle</b>' +
 					'</div>' +
 					'<div class="mw-collapsible-content">' + loremIpsum + '</div>' +
 				'</div>',
@@ -282,6 +282,22 @@
 		assert.assertTrue( $content.is( ':hidden' ), 'click event on non-link inside toggle toggles content' );
 	} );
 
+	QUnit.test( 'click on non-link inside toggler counts as trigger', function ( assert ) {
+		var $collapsible = prepareCollapsible(
+				'<div class="mw-collapsible">' +
+					'<div class="mw-collapsible-toggle">' +
+						'Toggle <a>toggle</a> toggle <b>toggle</b>' +
+					'</div>' +
+					'<div class="mw-collapsible-content">' + loremIpsum + '</div>' +
+				'</div>',
+				{ instantHide: true }
+			),
+			$content = $collapsible.find( '.mw-collapsible-content' );
+
+		$collapsible.find( '.mw-collapsible-toggle a' ).trigger( 'click' );
+		assert.assertTrue( $content.is( ':hidden' ), 'click event on link (with no href) inside toggle toggles content' );
+	} );
+
 	QUnit.test( 'collapse/expand text (data-collapsetext, data-expandtext)', function ( assert ) {
 		var $collapsible = prepareCollapsible(
 				'<div class="mw-collapsible" data-collapsetext="Collapse me!" data-expandtext="Expand me!">' +
-- 
2.20.1