Page MenuHomePhabricator

[Bug] Safari browser: Scroll position jumps when sticky header search toggle is clicked
Closed, ResolvedPublicBUG REPORT

Description

Steps to reproduce

  1. Using the Safari browser (I have not been able to replicate in any other browser), visit https://en.wikipedia.org/wiki/Barack_Obama?useskinversion=2&vectorstickyheader=1
  2. Scroll down the page to make the sticky header appear and then continue scrolling for a bit
  3. Click the search toggle icon

Note: You may need to repeat steps 2 and 3 for this bug to surface. For whatever reason, the scroll position seems to jump more frequently after scrolling and then clicking the search toggle vs. clicking the search toggle without prior scrolling although I've seen it happen in either scenario.

Expected results

  • Scroll position is maintained after the search toggle is clicked.

Actual results

  • Scroll position jumps after the search toggle is clicked.

https://jumpshare.com/v/L9Fd74BgJdYvRhZGUwRB

Environments observed

  • Browser version: Version 14.1.2 (16611.3.10.1.6)
  • OS version: Mac OS Big Sur 11.6
  • Device model: MacBook Pro
  • Device language: En

Check any additional observations

Developer Notes

Note, I've only been able to replicate this using Safari so something specific to that browser might be causing this.

It's a bit of mystery right now why this is happening. At first I thought this might have something to do with trying to focus an element that isn't visible yet, but even if I change the code locally to always show the search input and delay the focus by 1 second, the jump still occurs: https://jumpshare.com/v/CoqsNSgPC38VqYnWdr3g

QA Steps

First review the video in https://jumpshare.com/v/L9Fd74BgJdYvRhZGUwRB to understand how to (potentially) replicate this bug. It is important to scroll and then click the magnifying glass icon rather than clicking the magnifying glass icon without prior scrolling.

Given that the merged code could potentially affect other browsers, please perform the following steps in Safari, Edge, Firefox, and Chrome:

  1. Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog?useskinversion=2&vectorstickyheader=1
  2. Scroll down the page to make the sticky header appear and then continue scrolling for a bit
  3. Click the search toggle icon
  4. Verify that the browser's scroll position doesn't change and the bug doesn't surface
  5. Click the content area to toggle/make the magnifying glass icon appear again in the sticky header

Repeat steps 2 - 5 as many times as necessary to gain confidence that the bug doesn't appear (e.g. 5 times has been enough for me)

QA Results - Beta

ACStatusDetails
1T297636#7630962

QA Results - Prod

ACStatusDetails
1T297636#7630982

Event Timeline

Change 747183 had a related patch set uploaded (by Nray; author: Nray):

[mediawiki/skins/Vector@master] Reset scroll position when sticky header search input receives focus to fix Safari bug

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

Change 747183 abandoned by Nray:

[mediawiki/skins/Vector@master] Reset scroll position when sticky header search input receives focus to fix Safari bug

Reason:

Meant to be POC only, but this might be an option

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

I looked more into this today (including disabling the intersection observer), but everything was eventually a red herring which makes me think:

  1. This problem is unique to the Safari browser and possibly some unique permutation with our code
  2. Annoyingly, I haven't been able to make a minimum test case that would definitively prove what permutation leads to this bug, but I am extremely curious.

For whatever reason, Safari wants to change the scroll position when the search input is focused. Perhaps using the "preventScroll" option might help here, but Safari doesn't support the option yet.

Therefore, although hacky, we might want to consider the workaround as an option which seems to prevent the jump from happening in my testing.

I felt a bit uncomfortable with the proposed hack fix, as the issue is not fully understood and it's not clear when we could remove the hack, so this hack is likely going to stick around for a while with only this bug as context.

While testing, I noticed a related issue that clicking and holding on the line between search icon and title causes the article to scroll.

I want to explore other options before we go ahead with Nick's suggestion.

While testing, I noticed a related issue that clicking and holding on the line between search icon and title causes the article to scroll.

FWIW, that issue seems to be related to the use of scroll padding and also seems unique to Safari as I can't replicate it when the scroll padding is removed (and I can't replicate it in other browsers). Unfortunately though, the scroll position still jumps even without scroll padding in Safari:

https://jumpshare.com/v/gsEQsSef3AfNgBRfdod4

Change 747183 restored by Jdlrobson:

[mediawiki/skins/Vector@master] Reset scroll position when sticky header search input receives focus to fix Safari bug

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

Change 747183 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Reset scroll position when sticky header search input receives focus to fix Safari bug

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

nray updated the task description. (Show Details)

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Monterey
Browser: see steps
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Given that the merged code could potentially affect other browsers, please perform the following steps in Safari, Edge, Firefox, and Chrome:

  1. Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog?useskinversion=2&vectorstickyheader=1
  2. Scroll down the page to make the sticky header appear and then continue scrolling for a bit
  3. Click the search toggle icon
  4. Verify that the browser's scroll position doesn't change and the bug doesn't surface
  5. Click the content area to toggle/make the magnifying glass icon appear again in the sticky header

✅ AC1:

Safari:

Screen Recording 2022-01-18 at 5.23.07 PM.mov.gif (928×858 px, 2 MB)

Edge:

Screen Recording 2022-01-18 at 5.28.24 PM.mov.gif (926×858 px, 2 MB)

Firefox:

Screen Recording 2022-01-18 at 5.27.12 PM.mov.gif (924×858 px, 3 MB)

Chrome:

Screen Recording 2022-01-18 at 5.30.28 PM.mov.gif (926×858 px, 2 MB)

Edtadros subscribed.

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Monterey
Browser: see steps
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Given that the merged code could potentially affect other browsers, please perform the following steps in Safari, Edge, Firefox, and Chrome:

  1. Login and visit https://en.wikipedia.org/wiki/Dog?useskinversion=2&vectorstickyheader=1
  2. Scroll down the page to make the sticky header appear and then continue scrolling for a bit
  3. Click the search toggle icon
  4. Verify that the browser's scroll position doesn't change and the bug doesn't surface
  5. Click the content area to toggle/make the magnifying glass icon appear again in the sticky header

✅ AC1:

Safari:

Screen Recording 2022-01-18 at 5.38.29 PM.mov.gif (926×858 px, 2 MB)

Edge:

Screen Recording 2022-01-18 at 5.40.22 PM.mov.gif (926×858 px, 2 MB)

Firefox:

Screen Recording 2022-01-18 at 5.42.06 PM.mov.gif (926×858 px, 3 MB)

Chrome:

Screen Recording 2022-01-18 at 5.43.25 PM.mov.gif (926×858 px, 2 MB)

Looks good, resolving