Page MenuHomePhabricator

Self link with anchor does not jump to section on page
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue:

  • Go to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry while logged out (just for reproducability; this is an issue regardless of skin and I don't want to care about gadgets :).
  • Find the "see below" link in the first section in the sentence "An editor using multiple accounts for valid reasons should, on each account's user page, list all the other accounts with an explanation of their purpose (see below)."
  • Click below.

Observe: Jump to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry#Alternative_account_notification

  • Now, hit the back button such that the anchor is removed from the URL (remove it from the URL and press enter if nothing else)
  • Scroll down to the Legitimate uses section
  • At the end of the bullet that starts with privacy, there is a link to "the notification section below".
  • Click it.

Observe: No jump

What should have happened instead?:
Jumps are good. :)

wikitext to HTML:
The source wikitext for the first link is [[#Alternative account notification|below]] and the HTML is

<a href="#Alternative_account_notification">below</a>

The source wikitext for the second link is [[Wikipedia:Sockpuppetry#Alternative account notification|the notification section below]] and the HTML is

<a class="mw-selflink-fragment" href="#Alternative account notification">the notification section below</a>

Note the spaces in the fragment.

This should be URL encoded.

archive.org has no version from before today where this link doesn't do what is expected (here's January 19th's copy, note the underscores in the relevant link), so is there something in today's train that potentially caused the issue or is archive.org guessing at the intent of the spaces in the href?

Software version: 1.40.0-wmf.19 (3ee932e) 02:18, 17 January 2023

QA Results - Prod

ACStatusDetails
1T327467#8576651

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

And not to focus on the bug report itself, but should [[#fragment]] not also generate the class that was the point of the change?

thiemowmde added subscribers: Edtadros, thiemowmde, Lena_WMDE.

Yes and no. I can easily reproduce the issue locally, did a git bisect and was able to track the issue down to patch https://gerrit.wikimedia.org/r/876233. It's indeed linked to T198652: [regression] Links to anchors within the same page trigger page previews..

Note you get different results when you do

  • [[Wikipedia:Sockpuppetry#Alternative account notification|the notification section below]] vs.
  • [[#Alternative account notification|the notification section below]]

Only the first link is broken (spaces not encoded as _) and only if the link points to the current page. Links to other pages work as expected.

Personally I'm really surprised to see a patch that changes parts of the behavior of the parser for an issue that can and should be fixed in the JavaScript code of the Page-Previews extension. All it needs to do is to check if the link points to the current page. The extension did this before. Why not add this code back?

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

[mediawiki/core@master] Self link fragments should be properly escaped

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

@Jdlrobson is this from your self-link changes? /cc @cscott

Yep we forgot to escape it. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/882697

Personally I'm really surprised to see a patch that changes parts of the behavior of the parser for an issue that can and should be fixed in the JavaScript code of the Page-Previews extension. All it needs to do is to check if the link points to the current page. The extension did this before. Why not add this code back?

Identifying hash fragments as self links cheaply is really useful from a product perspective. There are applications outside Page previews.

Change 882697 merged by jenkins-bot:

[mediawiki/core@master] Self link fragments should be properly escaped

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

You will be able to QA this in production later today (check https://en.wikipedia.org/wiki/Special:Version is on 1.40.0-wmf.20)

Jdlrobson triaged this task as Medium priority.Jan 26 2023, 7:45 PM

Test Result - Prod

Status:
Environment: enwiki
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Go to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry while logged out
Find the "see below" link in the first section in the sentence "An editor using multiple accounts for valid reasons should, on each account's user page, list all the other accounts with an explanation of their purpose (see below)."
Click below.
Observe: Jump to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry#Alternative_account_notification

Now, hit the back button such that the anchor is removed from the URL (remove it from the URL and press enter if nothing else)
Scroll down to the Legitimate uses section
At the end of the bullet that starts with privacy, there is a link to "the notification section below".
Click it.
❌ AC1: Link should jump to the "Alternative account notification section."
@Jdlrobson, it looks like there are still spaces in the URL.

Screen Recording 2023-01-28 at 5.20.14 PM.mov.gif (994×1 px, 672 KB)

Jdlrobson moved this task from Needs More Work to QA on the Web-Team FY2022-23 Q3 Sprint 1 board.

Probably a caching issue as it's working for me now. Can you please try again?

Test Result - Prod

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

Test Artifact(s):

QA Steps

Go to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry while logged out
Find the "see below" link in the first section in the sentence "An editor using multiple accounts for valid reasons should, on each account's user page, list all the other accounts with an explanation of their purpose (see below)."
Click below.
Observe: Jump to https://en.wikipedia.org/wiki/Wikipedia:Sockpuppetry#Alternative_account_notification

Now, hit the back button such that the anchor is removed from the URL (remove it from the URL and press enter if nothing else)
Scroll down to the Legitimate uses section
At the end of the bullet that starts with privacy, there is a link to "the notification section below".
Click it.
✅ AC1: Link should jump to the "Alternative account notification section."

2.mov.gif (464×640 px, 2 MB)