Popups makes API queries for external links like
<a class="external text" href="https://git.wikimedia.org/tree/mediawiki%2Fextensions%2FFlow">
, and then ext.popups.renderer.article.js
errors on line 60 accessing re.query.pages.
Hover over an external link on mediawiki.org with console open, see this error, and in Net tab notice the API request
note titles is empty, so it's a pointless API request.
The fix could involve:
- Check link.data( 'title' ) is not empty. Better yet (?), don't make the API request if mw.Title.newFromText( title ) === null,
since that will be null for a blank or otherwise invalid title, and an invalid title is unlikely to have useful extracts and pageimages info.
- Test re.query before reaching inside it.
- Maybe Popups could skip external links altogether, either by looking at href or by adding ".external" to IGNORE_CLASSES or both. I don't know if an external link on a wiki page could have a title attr that's meaningful in the current wiki.
But I'm confused because the test for whether or not to show a popup seems duplicated in setupTriggers(), removeTooltips(), and mw.popups.render.render(), and setupTriggers() already intends to ignore links with empty titles. The find() in setupTriggers() searches for, in part,
a:not([title=""])
but that excludes links with empty titles while including links that have no title at all (such as external links), so:
- I think setupTriggers() should find() 'a' + notSelector + '[title]:not([title=""])'
This entire selector string should be set up once in mw.popups.SELECTOR, and shared with removeTooltips() instead of the implementation detail IGNORE_CLASSES.
Also see my comment on bug 65425.
To avoid these bugs ext.popups should eventually have a set of qunit tests that assert all the kinds of links it should ignore and all the ones which it should process.
Version: master
Severity: normal