[Bug] Exiting and re-entering the image gallery via browser history loses page scroll offset
Open, LowPublic
Actions

Assigned To

None

Authored By

	• Niedzielski
	Dec 4 2018, 11:45 PM

Description

Note: The scroll restoration works perfectly if you open the overlay once, but dare you open it a second time...
It also only impacts users who use the browser navigation controls. Clicking "X" restores scroll position every time.

Related: T209418

Steps to Reproduce (1)

Visit https://en.m.wikipedia.org/wiki/Charles_Darwin?oldid=871874243 in the MinervaNeue mobile skin on desktop browser
Scroll down to an image deep in the article such as the evolutionary tree diagram
Tap the image
Press browser back button
Press browser Forward button
Press browser back button

Steps to Reproduce (2)

Visit https://en.m.wikipedia.org/wiki/Charles_Darwin in the MinervaNeue mobile skin
Scroll down to an image deep in the article such as the evolutionary tree diagram
Tap the image
Tap details
Press browser back button twice

Expected Results

Scroll offset in page is unchanged

Actual Results

Scroll location is reset to the top

Environments Observed

enwiki prod

Browser Version

Chrome v71.0.3578.57

OS Version

Chrome OS v11151.33.0

Device Model

Pixel Slate

Device Language

English

Developer notes

When an Overlay is shown using Overlay.prototype.show window.pageYOffset is stored locally in a variable called scrollTop
When hide is executed, that scrollTop is passed back and restored in the hide function.
- Note it is also set inside the OverlayManager (_checkRoute method)
On clicking the image, this.scrollTop is set correctly
On clicking back, this.scrollTop is restored.
When clicking forward, this.scrollTop is set incorrectly as "1"

Removing the handling in checkRoute fixes this problem at the cost of breaking things not using the OverlayManager.
It doesn't look like any of our Overlay's are not using the OverlayManager so it should be safe to remove this code altogether \o/

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Jdlrobson	T214647 [EPIC] Re-define the contract for displaying drawers and overlays in MobileFrontend
		Open		None	T211162 [Bug] Exiting and re-entering the image gallery via browser history loses page scroll offset

Event Timeline

• Niedzielski created this task.Dec 4 2018, 11:45 PM

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptDec 4 2018, 11:45 PM

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

ovasileva moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.Dec 5 2018, 12:01 PM

• alexhollender_WMF mentioned this in T209418: When I navigate away from an article and then back, I'd like my scroll position to be preserved.Dec 10 2018, 5:29 PM

Confirming this bug (although on iOS/Chrome & Safari I am unable to reproduce it). As far as I can tell this only occurs as the result of using the browsers "forward" button to enter into the media overlay. I would guess this isn't a particularly common flow, and thus this is lower priority, but perhaps we could confirm with data?

A related bug that seems to be occurring on Chrome: if, from article A, I follow the "Steps to Reproduce (1)" above, then tap a blue link (going to article B), then tap the browser's "back" button (returning to article A), I return to a different part of article A than I departed from. This is somewhat described in T209418, although I was unable to reproduce that bug by itself.

Regarding ""Steps to Reproduce (2)", I noticed that if at the end of those steps I then press the browsers "forward" button again, the "Details" button has black text (instead of white). This seems rather trivial, I am noting it mainly incase it is some sort've clue as to what's going on in general here.

IMG_88D143D00E60-1.jpeg (1×750 px, 804 KB)

@Niedzielski can you clarify in the reproduction steps in the task's description when you mean the browser's back/forward button, versus the back/forward controls on the media overlay?

can you clarify in the reproduction steps in the task's description when you mean the browser's back/forward button, versus the back/forward controls on the media overlay?

@alexhollender, the dedicated back and forward buttons in the browser chrome available on all webpages. Also, the back button on Android and equivalent back / forward gestures on some tablets.

Jdlrobson renamed this task from [Bug] Exiting the image gallery loses page scroll offset to [Bug] Exiting and re-entering the image gallery via browser history loses page scroll offset.Dec 13 2018, 1:09 AM

Jdlrobson updated the task description. (Show Details)

Jdlrobson updated the task description. (Show Details)Dec 13 2018, 1:12 AM

Possibly related to T189225

Jdlrobson updated the task description. (Show Details)Dec 18 2018, 8:05 PM

Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.

@nray while you are looking at the ImageOverlay could you take a look at if your changes in T216198 can or could fix this?

In T211162#5018909, @Jdlrobson wrote:

@nray while you are looking at the ImageOverlay could you take a look at if your changes in T216198 can or could fix this?

My changes don't touch this code so the bug is still there (was able to replicate with my patch). I suggest we work on this separately from my changes as mine are already very large

Cool just checking! Thanks for confirming the bug is still present!

I verified again today and this is still a problem.

Jdlrobson moved this task from Triaged but Future to Tracking on the Web-Team-Backlog board.Jul 31 2019, 8:18 PM

Jdlrobson edited projects, added Web-Team-Backlog (Tracking); removed Web-Team-Backlog.

Jdlrobson edited projects, added MobileFrontend (MobileFrontend and MinervaNeue architecture); removed MobileFrontend.

Jdlrobson added a parent task: T214647: [EPIC] Re-define the contract for displaying drawers and overlays in MobileFrontend.Jul 31 2019, 8:21 PM

Jdlrobson moved this task from Backlog to Drawers and Overlays and bears oh my on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.Jul 31 2019, 8:40 PM

I was looking into this today and I don't think the issue is our OverlayManager. I think the hashchange event associates the scroll position of 0 with that route, so resets the pageYOffset accordingly. I think this might be a browser bug (or how the spec works - Firefox has same problem), so trying to work out a minimum test case.

If you throw an error at the start of the hashchange event for the 3rd history change, window.pageYOffset is already 0 and the window has scrolled before any overlay has been displayed... to be investigated later.

Jdlrobson lowered the priority of this task from Medium to Low.Aug 7 2019, 11:43 PM

Jdlrobson moved this task from Untriaged to Move to Backlog on the Web-Team-Backlog (Tracking) board.Sep 30 2021, 9:23 PM

Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptSep 30 2021, 9:23 PM

Jdlrobson edited projects, added Web-Team-Backlog; removed Web-Team-Backlog (Tracking).Nov 5 2021, 12:12 AM

ovasileva moved this task from Incoming to Groomed on the Web-Team-Backlog board.Nov 11 2021, 10:48 AM

Jdlrobson removed a project: MobileFrontend (MobileFrontend and MinervaNeue architecture).Nov 30 2021, 5:54 PM

Jdlrobson added a project: MobileFrontend.

	F27496507: en.m.wikipedia.org_wiki_Charles_Darwin(Pixel 2) (4).png
	Dec 11 2018, 2:58 PM

	F27496482: en.m.wikipedia.org_wiki_Charles_Darwin(Pixel 2).png
	Dec 11 2018, 2:58 PM

	F27496485: en.m.wikipedia.org_wiki_Charles_Darwin(Pixel 2) (1).png
	Dec 11 2018, 2:58 PM

	F27496488: en.m.wikipedia.org_wiki_Charles_Darwin(Pixel 2) (2).png
	Dec 11 2018, 2:58 PM

	F27496492: en.m.wikipedia.org_wiki_Charles_Darwin(Pixel 2) (3).png
	Dec 11 2018, 2:58 PM

[Bug] Exiting and re-entering the image gallery via browser history loses page scroll offsetOpen, LowPublicActions