Fix linkTitleChangeListener

I2ecf575b introduced a regression where the linkTitleChangeListener
wouldn't destroy the active link's title if it had been dwelled on
within 300 ms of the previous active link being abandoned.

The LINK_ABANDON_END action is dispatched after 300 ms and ignored if
the active link has changed.

Change-Id: If0c16afd6ca1c44f0e7eed497f62f0190005a619
This commit is contained in:
Sam Smith 2016-11-30 14:50:38 +00:00
parent e49103061e
commit e17e209003
2 changed files with 76 additions and 14 deletions

View file

@ -12,23 +12,52 @@
mw.popups.changeListeners.linkTitle = function () {
var title;
return function ( prevState, state ) {
var $link;
/**
* 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
*/
function destroyTitleAttr( el ) {
var $el = $( el );
// Has the user dwelled on a link? If we've already removed its title
// attribute, then NOOP.
if ( state.preview.activeLink && !title ) {
$link = $( state.preview.activeLink );
if ( title ) {
return;
}
title = $link.attr( 'title' );
title = $el.attr( 'title' );
$link.attr( 'title', '' );
$el.attr( 'title', '' );
}
// Has the user abandoned the link?
} else if ( prevState && prevState.preview.activeLink ) {
$( prevState.preview.activeLink ).attr( 'title', title );
/**
* Restores the title attribute of the element.
*
* @param {Element} el
*/
function restoreTitleAttr( el ) {
$( el ).attr( 'title', title );
title = undefined;
title = undefined;
}
return function ( prevState, state ) {
var hasPrevActiveLink = prevState && prevState.preview.activeLink;
if ( hasPrevActiveLink ) {
// Has the user dwelled on a link immediately after abandoning another
// (remembering that the LINK_ABANDON_END action is delayed by
// ~10e2 ms).
if ( prevState.preview.activeLink !== state.preview.activeLink ) {
restoreTitleAttr( prevState.preview.activeLink );
}
}
if ( state.preview.activeLink ) {
destroyTitleAttr( state.preview.activeLink );
}
};
};

View file

@ -42,15 +42,13 @@
// Does the change listener guard against receiving many state tree updates
// with the same activeLink property?
nextState = $.extend( {}, this.state, {
isDelayingFetch: false
} );
nextState = $.extend( true, {}, this.state );
this.linkTitleChangeListener( this.state, nextState );
this.state = nextState;
nextState = $.extend( {}, this.state );
nextState = $.extend( true, {}, this.state );
delete nextState.preview.activeLink;
this.linkTitleChangeListener( this.state, nextState );
@ -58,4 +56,39 @@
assert.strictEqual( this.$link.attr( 'title' ), 'Foo' );
} );
QUnit.test( 'it should restore the title when the user dwells on another link immediately', function ( assert ) {
var nextState,
$anotherLink = $( '<a title="Bar">' );
assert.expect( 3 );
this.whenTheLinkIsDwelledUpon();
nextState = $.extend( true, {}, this.state, {
preview: {
activeLink: $anotherLink
}
} );
this.linkTitleChangeListener( this.state, nextState );
assert.strictEqual( this.$link.attr( 'title' ), 'Foo' );
assert.strictEqual( $anotherLink.attr( 'title' ), '' );
// ---
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.'
);
} );
}( mediaWiki, jQuery ) );