Revert "Remove title attributes at init"

This reverts commit 6bc2077ed5.

The change causes issues with various popular gadgets on Wikimedia
wikis. The 'title' attributes have been the easiest way to determine
the target pages of links, and many gadgets have come to rely on that.

Bug: T269297
Bug: T269873
Change-Id: I49d315a13c327a1c5af51d3de887c0c17642e9fe

package.json

@@ -67,7 +67,7 @@
 	"bundlesize": [
 		{
 			"path": "resources/dist/index.js",
-			"maxSize": "13.8kB"
+			"maxSize": "13.9kB"
 		}
 	]
 }

src/changeListeners/index.js

@@ -1,5 +1,6 @@
 import footerLink from './footerLink';
 import eventLogging from './eventLogging';
+import linkTitle from './linkTitle';
 import pageviews from './pageviews';
 import render from './render';
 import settings from './settings';
@@ -9,6 +10,7 @@ import syncUserSettings from './syncUserSettings';
 export default {
 	footerLink,
 	eventLogging,
+	linkTitle,
 	pageviews,
 	render,
 	settings,

src/changeListeners/linkTitle.js

@@ -0,0 +1,66 @@
+/**
+ * Creates an instance of the link title change listener.
+ *
+ * While the user dwells on a link, then it becomes the active link. The
+ * change listener will remove a link's `title` attribute while it's the
+ * active link.
+ *
+ * @return {ext.popups.ChangeListener}
+ */
+export default function linkTitle() {
+	let title;
+
+	/**
+	 * Destroys the title attribute of the element, storing its value in local
+	 * state so that it can be restored later (see `restoreTitleAttr`).
+	 *
+	 * @param {Element} el
+	 * @return {void}
+	 */
+	function destroyTitleAttr( el ) {
+		const $el = $( el );
+
+		// Has the user dwelled on a link? If we've already removed its title
+		// attribute, then NOOP.
+		if ( title ) {
+			return;
+		}
+
+		title = $el.attr( 'title' );
+
+		$el.attr( 'title', '' );
+	}
+
+	/**
+	 * Restores the title attribute of the element.
+	 *
+	 * @param {Element} el
+	 * @return {void}
+	 */
+	function restoreTitleAttr( el ) {
+		$( el ).attr( 'title', title );
+
+		title = undefined;
+	}
+
+	return ( prevState, state ) => {
+		const hasPrevActiveLink = prevState && prevState.preview.activeLink;
+
+		if ( !state.preview.enabled ) {
+			return;
+		}
+
+		if ( hasPrevActiveLink ) {
+			// Has the user dwelled on a link immediately after abandoning another
+			// (remembering that the ABANDON_END action is delayed by
+			// ~100 ms).
+			if ( prevState.preview.activeLink !== state.preview.activeLink ) {
+				restoreTitleAttr( prevState.preview.activeLink );
+			}
+		}
+
+		if ( state.preview.activeLink ) {
+			destroyTitleAttr( state.preview.activeLink );
+		}
+	};
+}

src/index.js

@@ -131,6 +131,7 @@ function registerChangeListeners(
 	statsvTracker, eventLoggingTracker, pageviewTracker, callbackCurrentTimestamp
 ) {
 	registerChangeListener( store, changeListeners.footerLink( actions ) );
+	registerChangeListener( store, changeListeners.linkTitle() );
 	registerChangeListener( store, changeListeners.render( previewBehavior ) );
 	registerChangeListener(
 		store, changeListeners.statsv( registerActions, statsvTracker ) );
@@ -237,13 +238,11 @@ function registerChangeListeners(
 
 	rendererInit();
 
-	$( validLinkSelector )
-		.removeAttr( 'title' )
-
 	/*
 	 * Binding hover and click events to the eligible links to trigger actions
 	 */
-		.on( 'mouseover keyup', function ( event ) {
+	$( document )
+		.on( 'mouseover keyup', validLinkSelector, function ( event ) {
 			const mwTitle = titleFromElement( this, mw.config );
 			if ( !mwTitle ) {
 				return;
@@ -280,14 +279,14 @@ function registerChangeListeners(
 
 			boundActions.linkDwell( mwTitle, this, measures, gateway, generateToken, type );
 		} )
-		.on( 'mouseout blur', function () {
+		.on( 'mouseout blur', validLinkSelector, function () {
 			const mwTitle = titleFromElement( this, mw.config );
 
 			if ( mwTitle ) {
 				boundActions.abandon();
 			}
 		} )
-		.on( 'click', function () {
+		.on( 'click', validLinkSelector, function () {
 			const mwTitle = titleFromElement( this, mw.config );
 			if ( mwTitle ) {
 				if ( previewTypes.TYPE_PAGE === getPreviewType( this, mw.config, mwTitle ) ) {

tests/node-qunit/changeListeners/linkTitle.test.js

@@ -0,0 +1,100 @@
+import linkTitle from '../../../src/changeListeners/linkTitle';
+
+QUnit.module( 'ext.popups/changeListeners/linkTitle', {
+	beforeEach() {
+		this.$link = $( '<a>' )
+			.attr( 'title', 'Foo' );
+
+		this.linkTitleChangeListener = linkTitle();
+
+		this.state = {
+			preview: {
+				enabled: true,
+				activeLink: this.$link
+			}
+		};
+
+		// A helper method, which should make the following tests more easily
+		// readable.
+		this.whenTheLinkIsDwelledUpon = function () {
+			this.linkTitleChangeListener( undefined, this.state );
+		};
+	}
+} );
+
+QUnit.test( 'it should remove the title', function ( assert ) {
+	this.whenTheLinkIsDwelledUpon();
+
+	assert.strictEqual( this.$link.attr( 'title' ), '', 'The title is removed.' );
+} );
+
+QUnit.test( 'it shouldn\'t remove the title under certain conditions', function ( assert ) {
+	this.state.preview.enabled = false;
+
+	this.whenTheLinkIsDwelledUpon();
+
+	assert.strictEqual(
+		this.$link.attr( 'title' ),
+		'Foo',
+		'The title is not removed.'
+	);
+} );
+
+QUnit.test( 'it should restore the title', function ( assert ) {
+	this.whenTheLinkIsDwelledUpon();
+
+	// Does the change listener guard against receiving many state tree updates
+	// with the same activeLink property?
+	let nextState = $.extend( true, {}, this.state );
+
+	this.linkTitleChangeListener( this.state, nextState );
+
+	this.state = nextState;
+
+	nextState = $.extend( true, {}, this.state );
+	delete nextState.preview.activeLink;
+
+	this.linkTitleChangeListener( this.state, nextState );
+
+	assert.strictEqual(
+		this.$link.attr( 'title' ),
+		'Foo',
+		'The title is restored.'
+	);
+} );
+
+QUnit.test( 'it should restore the title when the user dwells on another link immediately', function ( assert ) {
+	const $anotherLink = $( '<a>' ).attr( 'title', 'Bar' );
+
+	this.whenTheLinkIsDwelledUpon();
+
+	let nextState = $.extend( true, {}, this.state, {
+		preview: {
+			activeLink: $anotherLink
+		}
+	} );
+
+	this.linkTitleChangeListener( this.state, nextState );
+
+	assert.strictEqual( this.$link.attr( 'title' ), 'Foo', 'The title is set.' );
+	assert.strictEqual(
+		$anotherLink.attr( 'title' ),
+		'',
+		'The title is removed.'
+	);
+
+	// ---
+
+	this.state = nextState;
+
+	nextState = $.extend( true, {}, nextState );
+	delete nextState.preview.activeLink;
+
+	this.linkTitleChangeListener( this.state, nextState );
+
+	assert.strictEqual(
+		$anotherLink.attr( 'title' ),
+		'Bar',
+		'It should restore the title of the other link.'
+	);
+} );

webpack.config.js

@@ -112,8 +112,8 @@ module.exports = ( env, argv ) => ( {
 		// Minified uncompressed size limits for chunks / assets and entrypoints. Keep these numbers
 		// up-to-date and rounded to the nearest 10th of a kibibyte so that code sizing costs are
 		// well understood. Related to bundlesize minified, gzipped compressed file size tests.
-		maxAssetSize: 41.8 * 1024,
-		maxEntrypointSize: 41.8 * 1024,
+		maxAssetSize: 42.2 * 1024,
+		maxEntrypointSize: 42.2 * 1024,
 
 		// The default filter excludes map files but we rename ours.
 		assetFilter: ( filename ) => !filename.endsWith( srcMapExt )