Keep current scroll position when zooming the document

[Snuffleupagus]

This PR is an attempt to rewrite #3446 from the ground up, to hopefully be able to pinpoint and fix the bugs in that implementation.

Fixes #1521 and replaces #3446.

/cc @brendandahl

[brendandahl]

/botio-windows preview

[pdfjsbot]

From: Bot.io (Windows)

---

Received

Command cmd_preview from @brendandahl received. Current queue size: 0

Live output at: http://107.22.172.223:8877/7cf813b415a8d66/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Success

Full output at http://107.22.172.223:8877/7cf813b415a8d66/output.txt

Total script time: 0.33 mins

Published

• Viewer: http://107.22.172.223:8877/7cf813b415a8d66/web/viewer.html
• Extension: http://107.22.172.223:8877/7cf813b415a8d66/extensions/firefox/pdf.js.xpi
• Extension (AMO): http://107.22.172.223:8877/7cf813b415a8d66/extensions/firefox/pdf.js.amo.xpi

[brendandahl]

I don't seem to be seeing any of the issues as before. Squash, then I'll merge.

[Snuffleupagus]

I don't seem to be seeing any of the issues as before. Squash, then I'll merge.

Great, that is a relief.

[brendandahl]

Entering and exiting full screen I was still getting stuck at the end of the document. This seems to fix that:

-    this.parseScale(this.presentationModeArgs.previousScale, true);
-    this.page = this.page;
+    var currentPage = this.pages[this.page - 1];
+    this.parseScale(this.presentationModeArgs.previousScale, true, true);
+    currentPage.scrollIntoView();

[brendandahl]

The same affect could also be had by moving the this.isPresentationMode = false; below the parse scale, but that seems kind of hacky.

[Snuffleupagus]

@brendandahl I've updated exitPresentationMode with your code snippet, thanks!
Obviously I wasn't able to reproduce that issue either, but this code works just as well in my testing so hopefully this will be fine. I also added a comment to prevent someone from just changing this back in an effort to simplify the code.

I also found that there where issues with scrolling the initial position of the document into view when reloading the page, if the user had specified a hash (i.e. #disableRange=true or similar). I missed this, since it was fixed "for free" by the previous version of this patch. Hopefully fixing this won't cause other issues.

Finally, I want to apologise for all the problems that this patch has caused! I obviously wouldn't have submitted it if it didn't work in my testing. (Also, I still cannot understand why I've been unable to reproduce even one of the issues you found!)

If this is OK, I'll squash the commits.

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux)

---

Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/4b5bdfc244b8ad7/output.txt

[pdfjsbot]

From: Bot.io (Linux)

---

Success

Full output at http://107.21.233.14:8877/4b5bdfc244b8ad7/output.txt

Total script time: 0.28 mins

Published

• Viewer: http://107.21.233.14:8877/4b5bdfc244b8ad7/web/viewer.html
• Extension: http://107.21.233.14:8877/4b5bdfc244b8ad7/extensions/firefox/pdf.js.xpi
• Extension (AMO): http://107.21.233.14:8877/4b5bdfc244b8ad7/extensions/firefox/pdf.js.amo.xpi

[Snuffleupagus]

@brendandahl Thanks for merging this, and please except my apologies for the inconvenience this patch caused you!

[brendandahl]

@Snuffleupagus it was no inconvenience, thanks for the feature!

[Snuffleupagus]

@brendandahl I'm really sorry, but I think we may need to back out this PR.
I've just noticed a couple of issues (it would be ironic if you cannot reproduce any of them):

• When exiting presentation mode, the right page will be scrolled into view, but nothing will be drawn (the page stays blank). To reproduce this issue:

  • Open http://mozilla.github.io/pdf.js/web/viewer.html and wait for the document to finish loading.
  • Enter presentation mode.
  • Change page a couple of times (at least three seemed enough in my testing).
  • Exit presentation mode.

  For me, this leads to the right page being scrolled into view, but remaining completely blank. This behaviour seem to be specific to Firefox, since in Chrome the page is rendered but ends up in the wrong scroll position.
  This might actually be solved by moving the line this.isPresentationMode = false; in exitPresentationMode (as we discussed on IRC, but rejected since it seemed to hacky). I'm not sure if this is enough to fix the issue completely though.

• When entering presentation mode before the document has finished loading and then going to the next page it will be blank. Unfortunately this issue is intermittent, making it really difficult to debug. So far I haven't been able to make any progress in figuring this out.

• There also seems to be some issues with this fix:

  I also found that there where issues with scrolling the initial position of the document into view when reloading the page, if the user had specified a hash (i.e. #disableRange=true or similar). I missed this, since it was fixed "for free" by the previous version of this patch. Hopefully fixing this won't cause other issues.

  This might lead to the page not being rendered when you reload the browser. This issue is also intermittent, as it seems to depend on the complexity of the page. I.e. a page with just text works fine, but anything with more complex graphics fails (page 11 of the tracemonkey paper always fail in my tests).
  This issue might be easy to fix with a hack, but a proper solution might not be too easy given the current state of setScale.

So to sum up the above: I don't really understand why some of these things happen and even the ones that I do sort of get, I'm not sure how to fix properly.
@brendandahl Therefore I would suggest backing out this PR, since I cannot promise when or even if I could fix these issues. I will certainly try, but I don't have high hopes given the trouble this has caused me already.

[timvandermeij]

@Snuffleupagus I hope you'll be able fix the issues and re-open this later, as it is great functionality. :)

[Snuffleupagus]

@Snuffleupagus I hope you'll be able fix the issues and re-open this later, as it is great functionality. :)

@timvandermeij Considering that I now know that this PR wasn't the sole culprit causing these issues, I'm willing to take a stab at implementing this again. But for this to happen, issue #3545 needs to be fixed first. Otherwise this PR would just be a waste of time, not to mention a great source of frustration ;-)