If a link points to another URL with a hash fragment, we show a preview for the other page.

@thiemowmde to support this we'd need to add classes to URI's that help us distinguish between "links to other pages" and "section links"

@Niedzielski my opinion is that a page preview of the page you're currently on is more confusing than helpful. I understand from the perspective of consistency that any link should have a corresponding page preview, however given the context (you are already on the page) I think the page preview a) doesn't add value to your experience, and b) potentially confuses you (for myself at least it usually takes me a few seconds to realize that I'm looking at a preview of the page I'm currently on). I am therefore okay with breaking the pattern in this case.

I don't know the history of why we don't show page previews for TOC links, however I wonder if the decision made to suppress those could add context to this discussion?

I feel more confident suggesting that in the case of the ^ indicators in the references section page previews are distracting and should be removed, than I am in making a global suggestion against all self-links. Calling out that @Lea_WMDE has worked planned on the references section, so perhaps it's worth pausing on this until they've come up with a plan.

Heyho,

Calling out that @Lea_WMDE has worked planned on the references section, so perhaps it's worth pausing on this until they've come up with a plan.

so our work in the references section (Being able to jump back to your reading postition) is done. The work on reference previews has just started, but I don't think I see page previews in the references section in the scope of the project. But please correct me if you think otherwise :)

Change 484698 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Popups@master] Also exclude the Cite extension's "Jump up" backlinks

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

Why is suppressing the preview better than showing a possibly redundant one?

This leaves me puzzled. What is the point of a popup showing the same page the user is currently reading? What new information does this reveal?

When I see a page preview popup, my expectation is that a click will "enlarge" this preview and "zoom in" on the previewed page. And indeed, this is what happens when the previewed page opens. But when the link points to the current page, there is nothing to enlarge, no new information to find, nothing new to discover. This feels like one of these infinite zooming GIFs that repeat the same information over and over again. Example: https://www.moillusions.com/infinite-zoom-coast-illusion/

to support this we'd need to add classes to URI's that help us distinguish between "links to other pages" and "section links"

It might be that I miss something. But from what I see all information is there:

• The frontend knows which page it currently shows. The best way to get this information is, I think mw.config.get( 'wgPageName' ).
• The Popups code knows the page titles of the page previews it is going to show.

All that needs to be done is to match these two pieces, and bail out if it's the same.

In the meantime I realized the issue with the ^ links got introduced via T206323 while the German-Community-Wishlist team worked on the Cite extension. I uploaded https://gerrit.wikimedia.org/r/484698. Note this will fix T212419, but not this ticket here.

Change 484698 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Also exclude the Cite extension's "Jump up" backlinks

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

I think the selector href~="#" is a bit too aggressive. If I link to [[Wikipedia#Community]], I would still want a preview of that page. Also, there's no way to distinguish between a link to [[Wikipedia#Community]] and [[Wikipedians]] with just a simple query selector like href~="#".

What I do think would be good to implement, as posted on https://www.mediawiki.org/wiki/Topic:V3nkf7urjggli4ut, would be either to block popups on href^="#" (because those are always links to the page you're already on), or if T156041 is implemented, as Quiddity mentions, at least restrict href="#".

Currently, this extension "conflicts" with the Kartographer extension, because it gives popups for the page you're on when you hover over the + or - zoom buttons, since those are links with a title attribute (which makes sense for what they use them for)

This just turned up in my inbox and I wanted to note, that the original problem described in the ticket does not exist anymore. At least I could not reproduce it. - I guess we fixed it while working on the reference previews.

This once again has started to happen, presumably for links with titles. I get such page previews in Convenient-Discussions for comment/section links of such form: <a tabindex="0" role="button" class="cd-comment-button-label cd-comment-timestamp cd-comment-button" href="#c-Lstanley563-20221018141700-Semi-protected_edit_request_on_18_October_2022" title="14:17, 18 October 2022 (UTC)">2 months ago</a>. I assume it's the title attribute that is the trigger.

Another example: at https://en.wikipedia.org/wiki/Help:Self_link,

• [[Help:Self link#Self-link to a section]] does create a page preview, while
• [[#Self-link to a section|Self-link to a section]] doesn't.

The example in the task https://en.wikipedia.beta.wmflabs.org/wiki/Self_link has regressed too, so I reopen this. (Or possibly another task needs to be created.)

Thanks for the report @Jack_who_built_the_house

We recently simplified page previews to only ignore links with the mw-selflink class. It looks like the link doesn't have the .mw-selflink class. Would it be problematic to add that class to links like this ie. is self link reserved for links without a hash fragment ?

(If it's problematic we should add a new class .mw-selflink-section and add that to the exclude selectors in page previews.)

Moving back to needs prioritization since this got reopened.

In T198652#8495666, @Jdlrobson wrote:

We recently simplified page previews to only ignore links with the mw-selflink class. It looks like the link doesn't have the .mw-selflink class. Would it be problematic to add that class to links like this ie. is self link reserved for links without a hash fragment ?

I can add, or course, the class mw-selflink to all same page links in Convenient-Discussions (and will add), but having all same page links with title to receive a popup seems to me to be not the most straightforward way. I mean, I can only guess the number of user scripts that would stumble upon this. (And MediaWiki itself since [[Help:Self link#Self-link to a section]] at https://en.wikipedia.org/wiki/Help:Self_link receives a popup.)

As you can see in rMGCDffe2497c07eb, this is not very pretty given you have to reset the styles for the usages of mw-selflink too. And God forbids that in a few years some dev adds an additional selector to a.mw-selflink CSS declaration...

Change 876233 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Add mw-selflink class to links to hash fragments in the same page

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

Hey @Jack_who_built_the_house sorry for the confusion. That's not what I meant. That would work on short term but long term I was wondering if we could do this:
https://gerrit.wikimedia.org/r/876233

@cscott is there any reason we couldn't do this?

Change 876235 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Parser: don't style selflinks for hash fragments the same as selflinks

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

Ah I see, OK 🙂

But still user script owners will have to add the mw-selflink/mw-selflink-fragment class, if I understand correctly.

Change 876235 abandoned by Jdlrobson:

[mediawiki/core@master] Parser: don't style selflinks for hash fragments the same as selflinks

Reason:

Merged into parent based on test.

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

Change 876250 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] Fixes: Links to anchors within the same page trigger page previews.

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

We recently simplified page previews […]

"Simplified". 😢️ Don't you think we added the isOwnPageAnchorLink and isSelfLink code for a reason? Actually a multitude of reasons, including T214861 and T215899, and T220097 for example. I remember us fine-tuning this code many times over the course of weeks. What was wrong? Are these bugs all back now?

We just realized the same thing happens on all kinds of UI elements that happen to be <a> elements. Diff pages for example have these little arrows to link moved paragraphs. These show popups now.

I remember working on the relevant code. Back then it was clear that it's not possible to make sure all <a> elements that could ever appear anywhere but should not show a popup can be identified via an exclude-list. You will always miss something. It really should be an include-list. Which can be as trivial as extracting the page name from the link (which must be done anyway) and checking if it's the current one.

I think what we also discovered was that JavaScript reports <a href="#foo"> as an absolute link (which kind of makes sense) to the current page. Other links in the HTML might already be expanded (e.g. <a href="/wiki/Foo#foo">). The really only thing we can be sure about is the page name.

Change 879058 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] Don't show page previews on hash fragments

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

Change 879061 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] Expand tests to include hash fragment behaviour

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

In T198652#8505401, @Jack_who_built_the_house wrote:

But still user script owners will have to add the mw-selflink/mw-selflink-fragment class, if I understand correctly.

So the latest version will mean script users either have to add mw-selflink, mw-selflink-fragment OR link without the path. E.g. if you are on page Foo a link to <a href="#section"> will not trigger a preview but <a href="Foo#section"> will. I think that's probably the most straightforward way to do this, and hopefully has the least impact on script editors?

We just realized the same thing happens on all kinds of UI elements that happen to be <a> elements. Diff pages for example have these little arrows to link moved paragraphs. These show popups now.

This will be fixed by the above proposed solution. We'll do that by excluding any link that begins with # character. We intentionally don't want to support expanded links as it results in for most cases unnecessary parsing of the URL that we can avoid. In the new system, the idea is the link should have enough information to tell whether a preview type should be triggered (without needing any other context like current page etc)

"Simplified". 😢️ Don't you think we added the isOwnPageAnchorLink and isSelfLink code for a reason?

@thiemowmde @phuedx and I did some work to support making page previews extensible and to assist a request a very longstanding request from the Math extension to add Math previews. We also wanted to land on an API that the web team could understand. We probably should have done this prior to reference previews in hindsight given web team wasn't actively involved in that project and Popups as a result mixes code from both our teams.

The patch was up for a long period of time (since July) and I found the self link code unnecessary complicated yes. I did assume that hash fragment code was added for a reason and I explicitly asked you about it here but unfortunately I wasn't able to get hold of you. In future I assume it would be better to ping you on Phabricator?

I did look at git blame and associated tickets which unfortunately did not describe the issue very well (no replication steps for example) and there were no unit tests explicitly covering it (or at least. no unit tests clearly failed when we made this change) so we made a misinformed decision. These things happen, we'll fix the issue and I think the overall outcome is good and the overall code for MediaWiki will be better for it.

Actually a multitude of reasons, including T214861 and T215899, and T220097 for example. I remember us fine-tuning this code many times over the course of weeks. What was wrong? Are these bugs all back now?

I've looked at those tickets and the refactor doesn't seem to have effected those, namely:

• reference previews work on https://en.wikipedia.beta.wmflabs.org/wiki/Dog?purge=now with extra parameters on the query string
• reference previews show up in all namespaces e.g. https://en.wikipedia.org/wiki/User:Jdlrobson/Nineteen_Eighty-Four
• reference previews work in a gendered namespace https://de.wikipedia.org/w/index.php?title=Benutzerin:Johanna_Strodt_(WMDE)/Spielwiese&oldid=187227016

If do you notice any other issue please raise a ticket and we'll get that fixed ASAP and I'll add some test cases so they don't break again.

Change 876233 merged by jenkins-bot:

[mediawiki/core@master] Parser: Add mw-selflink-fragment class to links to hash fragments in the same page

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

Change 876250 abandoned by Jdlrobson:

[mediawiki/extensions/Popups@master] Fixes: Links to anchors within the same page trigger page previews.

Reason:

See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/879058

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

Change 879058 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Don't show page previews on hash fragments

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

I think this one caused T327467: Self link with anchor does not jump to section on page.

there were no unit tests explicitly covering it […]

We tried hard to follow TDD back then, especially because it was not "our" codebase. Maybe not "test first" (I don't think anybody actually does this) but never leaving untested code behind. The codebase even enforces quite high coverage ratios. 90% for statements and lines, for example. I remember this being really helpful. We never decreased these ratios. This happened only much latter, it seems.

Unfortunately the recent patch effectively disabled the probably most relevant unit test.

In future I assume it would be better to ping you on Phabricator?

The relevant product manager for what was the Reference Previews project back then is @Lena_WMDE. As my team is currently working on the WMDE-GeoInfo-FocusArea we would need to re-assign resources. Email usually works best.

Personally I try hard to respond to specific questions about code on all channels including Gerrit. But I only have so much wiggle room and can't assist big rewrites outside of the promised team sprints.

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Visit https://en.wikipedia.beta.wmflabs.org/wiki/Self_link
Hover over "Section links"
✅ AC1: No page preview
This works now

Change 879061 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Expand tests to include hash fragment behaviour

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

Change #1047174 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Arlolra):

[mediawiki/core@master] Fix mw-selflink-fragment on variant fragment links

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

Change #1047174 merged by jenkins-bot:

[mediawiki/core@master] Fix mw-selflink-fragment on variant fragment links

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