Page MenuHomePhabricator

Media Viewer: Scroll position is lost on original page after accessing the original file
Open, LowPublic

Description

When a user clicks on an image in a page to open Media Viewer and then proceeds to view the original file, previous page position is lost when navigating back to the host page.

  1. Go to any article with images and scroll down a way.
  2. Click on any image.
  3. In Media Viewer, click the image again to go to the full-size version.
  4. Finish looking at the full-size version, so click browser "back" button and then "X" in MV, or click browser "back" button twice.
  5. You return to the top of the original page, not where you were scrolled to

Version: unspecified
Severity: normal

Details

Reference
bz71796

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:54 AM
bzimport added a project: MediaViewer.
bzimport set Reference to bz71796.
bzimport added a subscriber: Unknown Object (MLST).
Keegan created this task.Oct 8 2014, 2:05 PM

This is default browser behavior when reopening a hash link through the history. We can build something specific to act as a workaround, like storing the scroll position for a given hash in local storage, or making the scroll position part of the hash code.

Tgr added a comment.Oct 8 2014, 2:33 PM

This should not be the default behavior for at least some browsers, as they normally cache the page state in memory for history navigation ("bfcache"). We should figure out what in MediaViewer (or MediaWiki) breaks bfcache.

https://developer.mozilla.org/en-US/docs/Using_Firefox_1.5_caching
http://stackoverflow.com/questions/1195440/ajax-back-button-and-dom-updates/1195934#1195934

I think the cache is doing what it's supposed to. If you have media viewer open, the page scroll is 0. So at the time you move to another page, that's what the browser remembers and restores.

I.e. the browser can't be aware that media viewer "isn't really the page".

Tgr added a comment.Oct 8 2014, 2:36 PM

As for storing page location in localStorage, we are building hack upon hack to pretend we have a zoom function. If the current state is not acceptable, instead of further twiddling we should just suck it up and actually build one.

I didn't suggest that we should implement this hack, I'm just listing possibilities for posterity :) I personally think that trying to compensate for the browser's natural behavior isn't the way to go.

Tgr added a comment.Oct 8 2014, 2:40 PM

(In reply to Gilles Dubuc from comment #3)

I think the cache is doing what it's supposed to. If you have media viewer
open, the page scroll is 0. So at the time you move to another page, that's
what the browser remembers and restores.

MediaViewer stores the scroll position in a JS variable. If bfcache worked properly, upon backnavigation the browser would reload the page with MediaViewer open and the scroll position variable set, and MediaViewer uses that to restore the scroll position on close.

What actually happens is that the page is restored in its initial position, with MediaViewer closed; and the scroll position is reset due to the hash, so by the time MV opens and stores the scroll position it's zeroed out.

On the topic of relying on the browser, I guess then the solution is to close Media Viewer's UI like ninjas onbeforeunload, but without clearing the hash. This way when navigating back, the browser scroll cache kicks in, and the hash makes media viewer open.

Still an ugly hack... albeit a much faster one to implement.

Tgr added a comment.EditedOct 9 2014, 7:29 AM

Opened T73868: Support bfcache in MultimediaViewer about the bfcache issue.

Gilles triaged this task as Low priority.Nov 24 2014, 1:52 PM
Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:09 PM
Restricted Application added subscribers: Matanya, Aklapper. · View Herald TranscriptSep 4 2015, 6:09 PM

Mass-removing the Multimedia tag from MediaViewer tasks, as this is now being worked on by the Reading department, not Editing's Multimedia team.

Tgr removed a subscriber: Tgr.Tue, Jul 9, 6:03 PM