Fix a series of issues with misdetected reference elements

This installs a series of safety nets:

* The selector [href*="#"] skips links without a fragment.

* It's still possible that a fragment exists, but is empty.
mwTitle.getFragment() checks this.

* The gateway does not assume the element exists, but checks this first.
If there is no such element, the gateway aborts the request in a way
that no error popup is shown. This is currently only possible with the
`{ textStatus: 'abort', xhr: { readyState: 0 } }` response as seen in
this patch. We might need to introduce a new, more clean way to silently
quit a fetchPreviewForTitle() call.

* The test for the reference gateway finally covers the scraping code.

Bug: T214970
Bug: T214971
Change-Id: I9ec57e0fbb0d21beaaa7b359c1c2bef64d2c14f5

src/gateway/reference.js

@@ -16,7 +16,16 @@ export default function createReferenceGateway() {
 	 */
 	function fetchPreviewForTitle( title ) {
 		// Need to encode the fragment again as mw.Title returns it as decoded text
-		const id = title.getFragment().replace( / /g, '_' );
+		const id = title.getFragment().replace( / /g, '_' ),
+			$referenceText = $( `#${ $.escapeSelector( id ) } .reference-text` );
+
+		if ( !$referenceText.length ) {
+			return $.Deferred().reject(
+				'Footnote not found',
+				// Required to set `showNullPreview` to false and not open an error popup
+				{ textStatus: 'abort', xhr: { readyState: 0 } }
+			).promise( { abort() {} } );
+		}
 
 		return $.Deferred().resolve( {
 			// TODO: Provide different titles depending on the type of reference (e.g. "Book")
@@ -25,7 +34,7 @@ export default function createReferenceGateway() {
 			// TODO: Can probably be removed
 			// languageCode: 'en',
 			// languageDirection: 'ltr',
-			extract: $( `#${ $.escapeSelector( id ) } .reference-text` ).html(),
+			extract: $referenceText.html(),
 			type: previewTypes.TYPE_REFERENCE
 			// TODO: Can probably be removed
 			// thumbnail: '',

src/index.js

@@ -224,7 +224,7 @@ function registerChangeListeners(
 	const invalidLinksSelector = BLACKLISTED_LINKS.join( ', ' );
 	let validLinkSelector = `#mw-content-text a[href][title]:not(${ invalidLinksSelector })`;
 	if ( mw.config.get( 'wgPopupsReferencePreviews' ) ) {
-		validLinkSelector += ', .reference > a[href]';
+		validLinkSelector += ', .reference > a[href*="#"]';
 	}
 
 	rendererInit();
@@ -235,20 +235,23 @@ function registerChangeListeners(
 	$( document )
 		.on( 'mouseover keyup', validLinkSelector, function ( event ) {
 			const mwTitle = titleFromElement( this, mw.config );
+			if ( !mwTitle ) {
+				return;
+			}
+
 			let gateway = pagePreviewGateway;
 
 			if ( mw.config.get( 'wgPopupsReferencePreviews' ) ) {
-				// TODO: Can this condition be true for non-reference links?
-				if ( $( event.target ).parent().hasClass( 'reference' ) ) {
+				// The other selector can potentially pick up links with a class="reference" parent,
+				// but no fragment
+				if ( mwTitle.getFragment() &&
+					$( event.target ).parent().hasClass( 'reference' )
+				) {
 					gateway = referenceGateway;
 				}
 			}
 
-			if ( mwTitle ) {
-				boundActions.linkDwell(
-					mwTitle, this, event, gateway, generateToken
-				);
-			}
+			boundActions.linkDwell( mwTitle, this, event, gateway, generateToken );
 		} )
 		.on( 'mouseout blur', validLinkSelector, function () {
 			const mwTitle = titleFromElement( this, mw.config );

tests/node-qunit/gateway/reference.test.js

@@ -4,9 +4,17 @@ import createReferenceGateway from '../../../src/gateway/reference';
 QUnit.module( 'ext.popups/gateway/reference', {
 	beforeEach() {
 		mediaWiki.msg = ( key ) => `<${key}>`;
+
+		this.$references = $( '<ul>' ).append(
+			$( '<li id="cite_note--1">' ).append(
+				$( '<span class="reference-text">' ).text( 'Footnote' )
+			)
+		).appendTo( document.body );
 	},
 	afterEach() {
 		mediaWiki.msg = null;
+
+		this.$references.remove();
 	}
 } );
 
@@ -19,14 +27,24 @@ QUnit.test( 'Reference preview gateway returns the correct data', function ( ass
 			result,
 			{
 				url: '#cite_note--1',
-				// FIXME: The test should be set up in a way so this contains something.
-				extract: undefined,
+				extract: 'Footnote',
 				type: 'reference'
 			}
 		);
 	} );
 } );
 
+QUnit.test( 'Reference preview gateway rejects non-existing references', function ( assert ) {
+	const gateway = createReferenceGateway(),
+		title = createStubTitle( 1, 'Foo', 'undefined' );
+
+	return gateway.fetchPreviewForTitle( title ).then( () => {
+		assert.ok( false, 'It should not resolve' );
+	} ).catch( ( reason, result ) => {
+		assert.propEqual( result, { textStatus: 'abort', xhr: { readyState: 0 } } );
+	} );
+} );
+
 QUnit.test( 'Reference preview gateway is abortable', function ( assert ) {
 	const gateway = createReferenceGateway(),
 		title = createStubTitle( 1, 'Foo', 'cite note--1' ),