Revert "Fix tooltip interactions"

There is one reference to removeTooltips in 
ext.popups.targets.desktopTarget.

I suspect this got broken in a rebase. It seems Jenkins only runs
browser tests post merge. We should fix that.

Please resubmit the patch with this amended.

This reverts commit 0ff40a6532.

Change-Id: Idd8dffb853db760ebc5866190d008f173e3025ba

resources/ext.popups.core/ext.popups.core.js

@@ -36,35 +36,29 @@
 	];
 
 	/**
-	 * Temporarily remove the title attribute of a link so that
-	 * the tooltip doesn't show up alongside the Hovercard.
+	 * Temporarily remove the title attribute of the links so that
+	 * the yellow tooltips don't show up alongside the Hovercard.
 	 *
-	 * @method removeTooltip
-	 * @param {jQuery.Object} $link link from which to strip title
+	 * @method removeTooltips
 	 */
-	mw.popups.removeTooltip = function ( $link ) {
-		// We shouldn't empty the title attribute of links that
-		// can't have Hovercards, ie. TextExtracts didn't return
-		// anything. It's set in the article.init after attempting
-		// to make an API request.
-		if (
-			$link.data( 'dont-empty-title' ) !== true &&
-			$link.filter( '[title]:not([title=""])' ).length
-		) {
-			$link
-				.data( 'title', $link.attr( 'title' ) )
-				.attr( 'title', '' );
-		}
-	};
-
-	/**
-	 * Restore previously-removed title attribute.
-	 *
-	 * @method restoreTooltip
-	 * @param {jQuery.Object} $link link to which to restore title
-	 */
-	mw.popups.restoreTooltip = function ( $link ) {
-		$link.attr( 'title', $link.data( 'title' ) );
+	mw.popups.removeTooltips = function ( $elements ) {
+		$elements
+			.filter( '[title]:not([title=""])' )
+			.on( 'mouseenter focus', function () {
+				// We shouldn't empty the title attribute of links that
+				// can't have Hovercards, ie. TextExtracts didn't return
+				// anything. Its set in the article.init after attempting
+				// to make an API request.
+				if ( $( this ).data( 'dont-empty-title' ) !== true ) {
+					$( this )
+						.data( 'title', $( this ).attr( 'title' ) )
+						.attr( 'title', '' );
+				}
+			} )
+			.on( 'mouseleave blur', function () {
+				$( this )
+					.attr( 'title', $( this ).data( 'title' ) );
+			} );
 	};
 
 	/**

resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js

@@ -373,7 +373,8 @@
 			action: 'dwelledButAbandoned'
 		} ) );
 
-		$activeLink.off( 'mouseleave blur', leaveInactive );
+		// TODO: should `blur` also be here?
+		$activeLink.off( 'mouseleave', leaveInactive );
 		if ( openTimer ) {
 			openTimer.abort();
 		}

resources/ext.popups.targets.desktopTarget/desktopTarget.js

@@ -18,7 +18,7 @@
 		var $this = $( this ),
 			data = event.data || {};
 
-		$this.off( 'mouseleave.popups blur.popups', onLinkAbandon );
+		$this.off( 'mouseleave blur', onLinkAbandon );
 
 		mw.track( 'ext.popups.event', {
 			pageTitleHover: $this.attr( 'title' ),
@@ -55,25 +55,16 @@
 			mw.popups.removeTooltips( $link );
 			mw.popups.render.render( $link, event, eventData.linkInteractionToken );
 		} else {
-			// Remove existing handlers and replace with logging only
 			$link
-				.off( 'mouseleave.popups blur.popups mouseenter.popups focus.popups click.popups' )
-				// We are passing the same data, rather than a shared object, into two different
-				// functions so that one function doesn't change the data and
+				.off( 'mouseleave.popups blur.popups click.popups' )
+				// We are passing the same data, rather than a shared object, into two different functions.
+				// The reason is that we don't want one function to change the data and
 				// have a side-effect on the other function's data.
 				.on( 'mouseleave.popups blur.popups', eventData, onLinkAbandon )
 				.on( 'click.popups', eventData, onLinkClick );
 		}
 	}
 
-	/**
-	 * `mouseleave` and `blur` events handler for links that are eligible for
-	 * popups. Handles the restoration of title attributes
-	 */
-	function onLinkBlur() {
-		mw.popups.restoreTooltip( $( this ) );
-	}
-
 	/**
 	 * `click` event handler for links that are eligible for popups, but when
 	 * Popups are disabled.
@@ -86,7 +77,7 @@
 			href = $this.attr( 'href' ),
 			data = event.data || {};
 
-		$this.off( 'click.popups', onLinkClick );
+		$this.off( 'click', onLinkClick );
 
 		mw.track( 'ext.popups.event', {
 			pageTitleHover: $this.attr( 'title' ),
@@ -155,11 +146,11 @@
 	 * @method checkScroll
 	 */
 	mw.popups.checkScroll = function () {
-		$( window ).on( 'scroll.popups', function () {
+		$( window ).on( 'scroll', function () {
 			mw.popups.scrolled = true;
 		} );
 
-		$( window ).on( 'mousemove.popups', function () {
+		$( window ).on( 'mousemove', function () {
 			mw.popups.scrolled = false;
 		} );
 	};
@@ -175,8 +166,7 @@
 	function setupMouseEvents( $content ) {
 		mw.popups.$content = $content;
 		mw.popups.selectPopupElements()
-			.on( 'mouseenter.popups focus.popups', onLinkHover )
-			.on( 'mouseleave.popups blur.popups', onLinkBlur );
+			.on( 'mouseenter focus', onLinkHover );
 	}
 
 	mw.hook( 'wikipage.content' ).add( setupMouseEvents );

tests/qunit/ext.popups.core.test.js

@@ -33,9 +33,7 @@
 		}
 	} );
 
-	// FIXME: This test should be split to cover each function separately and a browser test should
-	// be created to test user interactions with focus and mouseenter - planned for the Hovercards rewrite
-	QUnit.test( 'removeTooltip and restoreTooltip', function ( assert ) {
+	QUnit.test( 'removeTooltips', function ( assert ) {
 		var $link = $( '<a>', {
 			text: 'link with tooltip',
 			title: 'link title',
@@ -44,20 +42,22 @@
 
 		QUnit.expect( 5 );
 
-		mw.popups.removeTooltip( $link );
-		assert.equal( $link.attr( 'title' ), '', 'The title should be removed by `removeTooltip`.' );
+		mw.popups.removeTooltips( $link );
 
-		mw.popups.restoreTooltip( $link );
-		assert.equal( $link.attr( 'title' ), 'link title', 'The title should be restored by `restoreTooltip`.' );
+		$link.trigger( 'mouseenter' );
+		assert.equal( $link.attr( 'title' ), '', 'The link does not have a title on mouseenter.' );
 
-		mw.popups.removeTooltip( $link );
-		assert.equal( $link.attr( 'title' ), '', 'Multiple calls should still remove title attribute' );
+		$link.trigger( 'mouseleave' );
+		assert.equal( $link.attr( 'title' ), 'link title', 'The link has a title on mouseleave.' );
 
-		mw.popups.restoreTooltip( $link );
-		assert.equal( $link.attr( 'title' ), 'link title', 'Multiple calls should still restore title attribute.' );
+		$link.trigger( 'focus' );
+		assert.equal( $link.attr( 'title' ), '', 'The link does not have a title on focus.' );
+
+		$link.trigger( 'blur' );
+		assert.equal( $link.attr( 'title' ), 'link title', 'The link has a title on blur.' );
 
 		$link.data( 'dont-empty-title', true );
-		mw.popups.removeTooltip( $link );
+		$link.trigger( 'mouseenter' );
 		assert.equal( $link.attr( 'title' ), 'link title',
 			'The link title is not removed when `dont-empty-title` data attribute is `true`.' );