Page MenuHomePhabricator

Various gadgets and extensions cause scroll position to shift after page load on Vector 2022 when linking to section
Closed, ResolvedPublic

Description

Various gadgets and extensions cause scroll position to shift after page load on Vector 2022.

While this is a problem with each gadget/extension, and not a bug in Vector 2022, this still affects the user experience in Vector 2022 compared to Vector 2010, which is somehow not affected. I think we should figure out why this happens and try to work around the problem in Vector 2022.

Ideally, each gadget/extension would be fixed to reserve space in just CSS, and then replace those placeholders with JS, so that page content would never shift due to the added interface elements. Shifting layout is also a poor user experience when linking to pages normally (see https://web.dev/cls/). But this is a lot of work.

Examples include:

QA Results - Beta

ACStatusDetails
1T330108#8639449

QA Results - Prod

ACStatusDetails
1T330108#8659034

Related Objects

Event Timeline

Browsers usually avoid this problem using something called "scroll anchoring". Basically, when you've scrolled down, the browser picks one of the elements visible on the screen, and if this element would move as a result of content being inserted elsewhere on the page, it adjusts the scroll position so that it remains in the same place on the screen. https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-anchor/Guide_to_scroll_anchoring

This works perfectly in old Vector, but seems to fail in new Vector.

The element gets chosen by a long algorithm: https://w3c.github.io/csswg-drafts/css-scroll-anchoring/#anchor-node-selection and there's no normal API to see what is picked, but Firefox has a secret debug feature that can highlight the anchor node: https://twitter.com/b56girard/status/1424851600045076503

Just got this protip from @ecbos_. Firefox pref 'layout.css.scroll-anchoring.highlight' lets you debug your scroll anchor. Getting this right is handy for virtualizing dynamic lists. Looks like Twitter anchors to the wrong place.

And it turns out that it picks the grid column containing the sticky TOC, which doesn't move as a result of these gadget changes, and so this choice doesn't help avoid the shifts:

image.png (2×3 px, 673 KB)

We can mark it with overflow-anchor: none https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-anchor so that it doesn't get picked as the anchor node. We also need to do the same for the other grid column on the right. And then we get a reasonable result – something in the content is the anchor node:

image.png (2×3 px, 663 KB)

The reason Vector 2022 is suffering from this, unike Vector 2010, is a combination of two changes:

  • The navigation was moved before the content in the DOM / HTML. Elements earlier in the DOM are preferred when picking the anchor node, so the navigation can be picked as long as it's visible on the screen.
  • The layout is done using CSS grid, instead of CSS 2 techniques like positioning or floats. Grid columns span the whole height of the page, and as a result they are always visible on the screen and viable candidates for the anchor node. (Note: this is not caused by position: sticky, as the behavior was the same when I removed it from the TOC/tools.)

Change 890515 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/skins/Vector@master] Disallow scroll anchoring in navigation grid columns

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

Of note. https://caniuse.com/?search=overflow-anchor is apparently not supported on Safari and/or iOS. @Jdlrobson sounds to me like a good topic for a Safari rant on Twitter ;)

Related tickets:
https://bugs.webkit.org/show_bug.cgi?id=171099 (scroll anchoring algo)
https://bugs.webkit.org/show_bug.cgi?id=109640 (overflow-anchor property)

Change 890515 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Disallow scroll anchoring in navigation grid columns

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

Edtadros subscribed.

Test Result - Beta

Status: ✅ PASS (per T330108#8645215)
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: page should not scroll when linking to a section.

Screen Recording 2023-02-22 at 5.00.25 PM.mov.gif (920×1 px, 1 MB)

Screen Recording 2023-02-22 at 5.05.32 PM.mov.gif (920×1 px, 988 KB)

@Jdlrobson I took a stab at the QA steps. It's really quick, but it does seem to scroll quickly. Another issue is if it's a nested TOC item, it won't expand the parent node. That may be the scope of another task. What are your thoughts on both issues?

Jdlrobson updated the task description. (Show Details)

@Edtadros those GIFs look like appropriate behaviour to me. My limited understanding of this bug is that when using a hash fragment, the referenced content should appear either at the top of page or just under the sticky header. For the purpose of QA, that's all that matters (you can ignore the table of contents)

For example in the Sdkb example the text "Having paid attention for a few days, the issue seems to occur when I click on notifications from conversations I'm subscribed to, not when I click on a table of contents. I'll follow up with the talk " should be highlighted and at the top of the page.

@matmarex @TheDJ is your patch meant to resolve all the issues in the description or is it a partial fix? Is there anything else that we should be QAing as part of this ticket?

It's in so far a partial fix, that it only works in Firefox and Chrome. And improving some of the gadgets a bit more might improve the experience on other platforms (and should probably happen regardless).

In other words, it should fix all of the issues, in supporting browsers.

I don't think you need any additional QA; I am hoping that some folks who reported the issues will confirm that they are fixed after the deployment.

I'd suggest that after closing this one, you could QA the parent tasks (T317967, T322837, T321221), since I think they should all be fixed (but I didn't carefully read through the discussion to figure out the exact requirements).

I'll check my repro (T321221), which currently still has the old behavior. Are you planning to roll this out to en.wikipedia first?

The fix will be deployed to all Wikimedia wikis next week on the usual schedule, including en.wikipedia on Thursday, 3 March.

Test Result - Prod

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

Test Artifact(s):

QA Steps

✅ AC1: page should not scroll when linking to a section.
It does take a while because I picked a very long page and linked to a section way at the bottom. No scrolling!

Screen Recording 2023-03-01 at 4.13.00 PM.mov.gif (786×1 px, 855 KB)

My version of this bug, T321221, seems fixed on en wikipedia today.

FYI. Safari now supports the scroll anchoring algo (in the new Safari version of sep 2023), https://bugs.webkit.org/show_bug.cgi?id=261628 but not yet overflow-anchor. I haven't had time to test this yet on my devices.