Inline code setting target="_blank" in renderReferencePreview()

As discussed in Iaadcce9. This does have a few benefits:
* Less code in the already pretty big render.js file.
* The code setting the target attribute is much closer to where it
  belongs: in the file that specifies how the content of a reference
  popup should look and behave.
* The class name "mwe-popups-extract" is not mentioned in two different
  files, but in the same.

Note this changes the signature of this src/ui/templates/… file to not
return an HTML string any more, but a jQuery object. The other templates
still return strings. I believe this is fine, and not that much of a
difference anyway. The signatures don't need to be identical. And the
jQuery object still represents the exact same HTML as before.

If it helps we could change all templates/… signatures accordingly.
Could be done in this or a separate patch.

Bug: T213908
Bug: T214970
Change-Id: I05ed4b886f79c5ae748f53ab9fed965dfd217620
This commit is contained in:
Thiemo Kreuz 2019-01-26 10:56:08 +01:00 committed by WMDE-Fisch
parent 870ddbb4a7
commit 67ceeaeeed
4 changed files with 13 additions and 15 deletions

Binary file not shown.

Binary file not shown.

View file

@ -244,19 +244,8 @@ function createDisambiguationPreview( model ) {
* @return {ext.popups.Preview} * @return {ext.popups.Preview}
*/ */
function createReferencePreview( model ) { function createReferencePreview( model ) {
const $el = $(
$.parseHTML( renderReferencePreview( model ) )
);
// Make sure to not destroy existing targets, if any
$el.find( '.mwe-popups-extract a[href]:not([target])' ).each( ( i, a ) => {
a.target = '_blank';
// Don't let the external site access and possibly manipulate window.opener.location
a.rel = `${ a.rel ? `${ a.rel } ` : '' }noopener`;
} );
return { return {
el: $el, el: renderReferencePreview( model ),
hasThumbnail: false, hasThumbnail: false,
isTall: false isTall: false
}; };

View file

@ -9,7 +9,7 @@ const mw = mediaWiki;
/** /**
* @param {ext.popups.PreviewModel} model * @param {ext.popups.PreviewModel} model
* @return {string} HTML string. * @return {jQuery}
*/ */
export function renderReferencePreview( export function renderReferencePreview(
model model
@ -18,7 +18,7 @@ export function renderReferencePreview(
url = escapeHTML( model.url ), url = escapeHTML( model.url ),
linkMsg = escapeHTML( mw.msg( 'popups-refpreview-jump-to-reference' ) ); linkMsg = escapeHTML( mw.msg( 'popups-refpreview-jump-to-reference' ) );
return renderPopup( model.type, const $el = $( $.parseHTML( renderPopup( model.type,
` `
<strong class='mwe-popups-title'> <strong class='mwe-popups-title'>
<span class='mw-ui-icon mw-ui-icon-element mw-ui-icon-preview-reference'></span> <span class='mw-ui-icon mw-ui-icon-element mw-ui-icon-preview-reference'></span>
@ -31,5 +31,14 @@ export function renderReferencePreview(
<a href='${ url }' class='mwe-popups-read-link'>${ linkMsg }</a> <a href='${ url }' class='mwe-popups-read-link'>${ linkMsg }</a>
</footer> </footer>
` `
); ) ) );
// Make sure to not destroy existing targets, if any
$el.find( '.mwe-popups-extract a[href]:not([target])' ).each( ( i, a ) => {
a.target = '_blank';
// Don't let the external site access and possibly manipulate window.opener.location
a.rel = `${ a.rel ? `${ a.rel } ` : '' }noopener`;
} );
return $el;
} }