Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remember view position after refreshing the page #6847

Closed
Rob--W opened this issue Jan 8, 2016 · 34 comments
Closed

Remember view position after refreshing the page #6847

Rob--W opened this issue Jan 8, 2016 · 34 comments
Labels
viewer

Comments

@Rob--W
Copy link
Member

Rob--W commented Jan 8, 2016

Currently the view position is saved by a hash based on the file contents. When the page reloads, we should also take the last position into account, because it matches the normal behavior of browsers. Usually, when you reload a web page, the scroll offset is restored (even if the content of the page changed).

The motivation for this change comes from experiencing the following broken workflow:

  1. Generate a local PDF file (file://....pdf).
  2. Open PDF with PDF.js and scroll to some chapter in the PDF file.
  3. Edit PDF file.
  4. Refresh the PDF.js viewer (e.g. with F5).
  5. Expected result: Retain the scroll position.
    Actual result: Page 1 is shown in the viewport.

Technical notes:

  • performance.navigation.type can be used to detect page reload versus navigations.
  • history.state is preserved when a page reloads.
@yashamon
Copy link

That would be awesome.

@Fatehsandhu
Copy link

I'm a student at Seneca college learning open source, and I was hoping to work on this bug for my course. If no one else is currently working on it, I'd like to give it a try.

@timvandermeij
Copy link
Contributor

Nobody indicated that they're working on it, so it's all yours! Feel free to contact us on IRC or leave a message here in case you have questions.

@Fatehsandhu
Copy link

Hey thanks a lot for the quick reply. I really want to contribute to an open source project. I will start working on it right away. Since this my first time doing something like this, is there anything that I should know about?
Thanks a a lot!

@timvandermeij
Copy link
Contributor

timvandermeij commented Oct 1, 2017 •
edited

I think all necessary information for this patch is listed on https://github.com/mozilla/pdf.js/wiki/Contributing. Unless you're touching files in the src/ folder (which I do not expect; I expect you'll only need to touch files in the web/ folder), you only need to run gulp lint and gulp unittest to verify your changes. You can run gulp server to start a local server to test your changes in the browser. If you have more questions, check the wiki, contact us on IRC or ask here. Good luck!

@Fatehsandhu
Copy link

Thanks, I will start reading through the files.

@andreip
Copy link

andreip commented Dec 30, 2017 •
edited

I'm looking into this but I don't know if I understood the problem very well.

1 - Generate a local PDF file (file://....pdf).
3 - Edit PDF file.

So the issue is only related to building/generating my own PDF? E.g. building it with some pdf generator like latex/jspdf?

I did the following and couldn't reproduce:

  1. Built myself a PDF and opened it with http://localhost:8888/web/viewer.html?file=/andrei_test/a4.pdf
  2. navigated to page 3.
  3. Then edited the pdf (added more text in page3)
  4. refreshed and saw the new content on page3 appear but I was still at page 3, pdf.js didn't move me to page1.

Before this, I just tried refreshing the default PDF from viewer.html a few times and I was under the impression that the page wasn't remembered at all. But now I think I understand, if I refresh it too fast (before the internal hashing is done to remember where to scroll back after refresh), then it will just get me to the last place I was before my last scroll, not to my last position. But if I wait half a second more and then refresh, then I see it's fine, I get scrolled position to where I last scrolled.

So I'm not really sure what I'm after here. Could you give more details onto how to reproduce? Thanks!

@yashamon
Copy link

yashamon commented Dec 31, 2017 via email

@yashamon
Copy link

yashamon commented Jan 9, 2018 via email

@BrianNgo
Copy link

I can confirm this problem is reproducible. Not only the page moved back, the zoom also got reset. I suspect this may be due to the hash got changed when we modified the file.

@Jolo510
Copy link

Jolo510 commented Jan 11, 2018

@timvandermeij Is this up for grabs? I'd like to take a crack at it!

@glittle2638
Copy link

I can't figure it out

@andreip
Copy link

andreip commented Jan 11, 2018

BrianNgo: I can confirm this problem is reproducible. Not only the page moved back, the zoom also got reset. I suspect this may be due to the hash got changed when we modified the file.

@BrianNgo Did you work on local, with the code, or how did you test this out? Could you give some step-by-step reproducing info?

yashamon: And still working remotely with http-server

@yashamon could you explain more your setup? It might be dependent on that, since when I tried running a local server and accessing it at localhost (e.g. http://localhost:8888/web/viewer.html?file=/andrei_test/a4.pdf), I couldn't reproduce this. I was also using chrome.

Jolo510: @timvandermeij Is this up for grabs? I'd like to take a crack at it!

@Jolo510 It's up for grabs, go for it. I'm not working on it, I couldn't reproduce it last time I tried.

@Rob--W
Copy link
Member Author

Rob--W commented Jan 11, 2018

The issue here is that the file changed only slightly, but the hash changed completely. For the purpose of testing, the actual PDF content does not matter, you only need to ensure that the PDFs that you are testing have different hashes.

To reproduce more reliably, you could take a set of completely unrelated PDF files (e.g. the PDFs in test/pdfs/), and overwrite a PDF file before reloading PDF.js (with the view set to page 2, so that you will see the difference between page 1 and page 2). In this way, the same file path will point to a different file and you can see the bug in action.

@BrianNgo
Copy link

BrianNgo commented Jan 11, 2018 •
edited

@andreip Yes, I am testing it on local with Chrome. What I did is open the pdf similar to what you had: http://localhost:8888/web/viewer.html?file=/andrei_test/a4.pdf. Then I used libreoffice to modify the file and exported it. Refreshed the page and the bug happened.

In my opinion, this is not really a bug. By modifying the file, the app perceives the current file as new file (which is the safest thing to assume). Thus the app should reset its history to view it as a new file.

The real issue would be when refreshing the file too quick before internal hashing is done.

@Jolo510
Copy link

Jolo510 commented Jan 12, 2018

@andreip Awesome! I'll see if I repo it locally.

@Jolo510
Copy link

Jolo510 commented Jan 18, 2018 •
edited

I plan on getting the app running locally tonight. Then make some time in the next day or two to reproduce the bug and dig into the code.

@BrianNgo If the issue is refreshing the file too quickly, what would be a potential fix?

@yashamon
Copy link

yashamon commented May 16, 2018 via email

@Jolo510
Copy link

Jolo510 commented May 17, 2018

@yashamon Nope, I haven't made any progress on this.

@ankitverma2211
Copy link

@Rob--W
hey ,
I see many people taking a shot at it . Giving it a try . Please let me know if we need to write test for this as well

@Rob--W
Copy link
Member Author

Rob--W commented Jul 27, 2018

@ankitverma2211 If possible, tests would be great.
However, we don't have automated tests for this kind of features, so if the patch looks reasonable and passes a manual test, then we would accept it too.

@rahsai374
Copy link

I would like to start on this. is anyone else currently working on this?

@timvandermeij
Copy link
Contributor

Not that I know of. Feel free to work on this!

@rahsai374
Copy link

rahsai374 commented Dec 24, 2018 via email •
edited

@rahsai374
Copy link

rahsai374 commented Dec 28, 2018 •
edited

@timvandermeij I have gone through the whole code which is involved while rendering pdf file.
It uses local storage for storing pdfjs view history with files as an array. In which each element stores the fingerprint of the file and other metadata about the last view history. when we modify a file fingerprint of the file changes and for that new fingerprint we don't have any view history.

my old file fingerprint => 14ecd8cdbbf6f76f04030d59025b5937

fingerprint after file change => 619c4c4f872e96e6514b25c6a1ae03f2

As far as I have gone through fingerprint calculation for a doc it depends on the content and pdf trailer.

here is some reference

figerprint calculation

stackoverflow referance

let me know what you say about this. should we close this issue?

@yashamon
Copy link

yashamon commented Dec 28, 2018 via email

@rahsai374
Copy link

it will be of great help if you can share the link to repo which is responsible for this and then it will be a feature to this repo rather than a bug

@xinhaoyuan
Copy link

xinhaoyuan commented Dec 30, 2018 •
edited

I used to have a greasemonkey script that replace key "C-r" to a "viewBookmark" click, which basically solves this issue for me. It didn't work after some version of Firefox. It looks like that greasemonkey is not loaded in pdf.js. Is it intended?

EDIT: after a bit of search I think this is intentional - https://discourse.mozilla.org/t/extensions-on-pdfjs-pages/28441

@rahsai374
Copy link

@timvandermeij @yashamon

I had look at Sharelatex repo. they are doing it using keeping track of pdfjs.history with projectId rather than fingerprint which changes with document changed, but projectId for that particular document remains same for sharelatex.

I have few questions in mind. I tried to connect with you guys in IRC but didn't got reponse

Questions:

  1. is we need to maintain page number also when pdf changes and the user opens a new file in a new tab.
    as it is maintained in the current fingerprint method.
  2. If it needs to be only in current tab we can use of sessions otherwise we will append some more keys to view_history.
    please guide me

@Snuffleupagus Snuffleupagus mentioned this issue Jan 6, 2019
Merged
@timvandermeij
Copy link
Contributor

Fixed in #10424.

@yashamon
Copy link

yashamon commented Jun 3, 2019 •
edited

Just tested this, still same behavior. Refreshing page fixes page view position only if the pdf file is unchanged, otherwise view jumps to first page. This is very easy to test with latex pick a document compile and preview the pdf add a random word in the latex source, recompile and preview the pdf, pdfjs preview jumps to first page. I am on release 2.2.191 in chrome. Will check in firefox when I get a chance.

@yashamon
Copy link

yashamon commented Jun 3, 2019

I tested with firefox, it looks like on the latest release the issue is fixed, so is it just that Chrome version is lagging behind?

@Rob--W
Copy link
Member Author

Rob--W commented Jun 4, 2019

The Chrome extension version includes this patch. Its behavior may differ because of a difference in how the browser behaves. I once posted a detailed description of the problem at cdea75d (which was part of #6200).

@timvandermeij timvandermeij mentioned this issue Nov 26, 2019
Closed
@allefeld
Copy link

I submitted a similar issue #11359 with respect to *latex-generated pdf. It is actually not correct that this uses a "hash based on the file contents" @Rob--W. Rather, it is an ID embedded in the PDF upon creation, and how that ID is generated depends on the generating application, for *latex it is a hash based on the combination of the current time and the pathname of the tex-file. See my last comment there for a solution.

@zamzterz zamzterz mentioned this issue Dec 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
viewer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests