Last modified: 2014-11-14 05:20:58 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T67929, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 65929 - Hovercards wastes API call and "TypeError re.query is undefined" on external links and other links with no title
Hovercards wastes API call and "TypeError re.query is undefined" on external ...
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Popups (Other open bugs)
master
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-30 02:24 UTC by spage
Modified: 2014-11-14 05:20 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description spage 2014-05-30 02:24:26 UTC
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

https://www.mediawiki.org/w/api.php?action=query&format=json&prop=extracts%7Cpageimages%7Crevisions%7Cinfo&redirects=true&exintro=true&exsentences=2&explaintext=true&piprop=thumbnail&pithumbsize=300&rvprop=timestamp&inprop=watched&indexpageids=true&titles=

note titles is empty, so it's a pointless API request.

The fix could involve:

1. 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.

2. Test re.query before reaching inside it.

3. 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:

4. 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.
Comment 1 Gerrit Notification Bot 2014-06-04 04:16:53 UTC
Change 137268 had a related patch set uploaded by Prtksxna:
core: Ignore '.external' links

https://gerrit.wikimedia.org/r/137268
Comment 2 Gerrit Notification Bot 2014-06-04 05:14:47 UTC
Change 134266 had a related patch set uploaded by Prtksxna:
core: Only run the selector for the a elements once

https://gerrit.wikimedia.org/r/134266
Comment 3 Gerrit Notification Bot 2014-07-15 16:00:48 UTC
Change 137268 merged by jenkins-bot:
core: Ignore '.external' links

https://gerrit.wikimedia.org/r/137268
Comment 4 Andre Klapper 2014-11-12 14:48:07 UTC
All patches mentioned in this report were merged or abandoned - is there more work left to do here (if yes: please reset the bug report status to NEW or ASSIGNED), or can you close this ticket as RESOLVED FIXED?

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links