Reference popups not working with unexpected URL parameters
Closed, ResolvedPublic5 Estimated Story Points
Actions

Assigned To

Authored By

	WMDE-Fisch
	Jan 28 2019, 6:09 PM

Description

Motivation
Article pages sometimes contain extra parameters, for example when you look at an older revision coming from the history.
Right now, reference previews do not work whenever the URL has any other parameter but the title parameter.

Bug example

https://en.wikipedia.beta.wmflabs.org/wiki/Dog?purge=now

This seems due to the way the title parser is used. It will return undefined in the above case.

https://gerrit.wikimedia.org/g/mediawiki/extensions/Popups/+/46cda9fa44c8aee99856606be89cc23187a7d94a/src/title.js#38

that clashes with

https://gerrit.wikimedia.org/g/mediawiki/extensions/Popups/+/46cda9fa44c8aee99856606be89cc23187a7d94a/src/index.js#237

Acceptance Criteria

No matter the extra parameters of the page URL, (even if it is more than one), reference previews work.

Notes

The behavior does not affect page previews.
It should be checked back with Web-Team-Backlog team how the current implementation came into existence.

Details

	Subject	Repo	Branch	Lines +/-
	Alternative way of detecting self-links	mediawiki/extensions/Popups	master	+9 -13
	Teach the title parser to always accept self-links	mediawiki/extensions/Popups	master	+67 -3

Customize query in gerrit

Related Objects

Mentioned In: T198652: [regression] Links to anchors within the same page trigger page previews.
Mentioned Here: T198652: [regression] Links to anchors within the same page trigger page previews.

Event Timeline

WMDE-Fisch created this task.Jan 28 2019, 6:09 PM

Lea_WMDE moved this task from Backlog to In preparation on the Reference Previews board.Jan 29 2019, 12:26 PM

Lea_WMDE updated the task description. (Show Details)Jan 29 2019, 2:10 PM

Lea_WMDE set the point value for this task to 5.

Lea_WMDE moved this task from In preparation to Ready for pickup on the Reference Previews board.

WMDE-Fisch merged a task: T215197: [Bug] References previews not shown for some URLs.Feb 5 2019, 1:33 PM

WMDE-Fisch added subscribers: • Niedzielski, Aklapper.

@Niedzielski do you know if there's a reason for that getTitle() does not return a valid title when there are other parameters in the link URL than title=?

See https://gerrit.wikimedia.org/g/mediawiki/extensions/Popups/+/ce2165fb96152134aee650b23a550cbb90de9584/src/title.js#43

In T214861#4927359, @WMDE-Fisch wrote:

@Niedzielski do you know if there's a reason for that getTitle() does not return a valid title when there are other parameters in the link URL than title=?

See https://gerrit.wikimedia.org/g/mediawiki/extensions/Popups/+/ce2165fb96152134aee650b23a550cbb90de9584/src/title.js#43

Ok I just found at least one reason and that's the [edit] links next to section headings. They would trigger a page preview popup ( to the own page though ). I wonder what other examples there are and if there's another way to avoid these things.

WMDE-Fisch added a project: WMDE-QWERTY-Sprint-2019-02-06.Feb 6 2019, 1:42 PM

WMDE-Fisch moved this task from Ready for pickup to Doing on the Reference Previews board.

@Niedzielski do you know if there's a reason for that getTitle() does not return a valid title when there are other parameters in the link URL than title=?

I do not know. There's a test case for multiple query parameters but no explanation. Perhaps, it's to avoid showing previews for any usage of the action API. E.g., https://en.wikipedia.org/w/index.php?title=Barack_Obama&action=history.

Change 489746 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Popups@master] Teach the title parser to always accept self-links

https://gerrit.wikimedia.org/r/489746

gerritbot added a project: Patch-For-Review.Feb 11 2019, 6:05 PM

I explored a bunch of ideas to get a grip on this issue. Finding some solution isn't that hard, but I want a solution that is sustainable and guaranteed to not mess with regular page previews. It should especially not make T198652 worse.

As far as I can tell the title parser was intentionally build as restricted as it is. It should only accept links it absolutely must accept.
One idea I had was to create a whitelist of URL parameters the title parser is allowed to ignore. We would need a list of use-cases where we want reference previews to be shown, and a (hopefully) complete list of use-cases where we do not want page previews to be shown. I tried, but this gets somewhat bonkers and unmaintainable pretty quickly.
I believe the gold solution is to detect self-links, as discussed in T198652.

The patch https://gerrit.wikimedia.org/r/489746 tries to keep the best of both worlds: The title parser is still very strict and only accepts links it absolutely must accept. Self-links are only detected as self-links when they are really identical. No parameters are ignored. Even their order is relevant. Links like the [edit] section links are not self-links, even if they point to the same title.

Change 491002 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Popups@master] [WIP] Alternative way of detecting self-links

https://gerrit.wikimedia.org/r/491002

Change 489746 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Teach the title parser to always accept self-links

https://gerrit.wikimedia.org/r/489746

WMDE-Fisch moved this task from Review to Demo on the WMDE-QWERTY-Sprint-2019-02-06 board.Feb 19 2019, 1:39 PM

Lea_WMDE closed this task as Resolved.Feb 20 2019, 9:09 AM

Lea_WMDE claimed this task.

Lea_WMDE moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2019-02-06 board.

Change 491002 abandoned by Thiemo Kreuz (WMDE):
Alternative way of detecting self-links

Reason:
The effectively only reason for this possible rewrite have been concerns with the browser compatibility, namely Internet Explorer 11 . This doesn't seem to be an issue.