OOP PDF: Fix incorrect scrolling during fit-to-page

chrome/browser/resources/pdf/viewport.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * Returns the height of the intersection of two rectangles.
  * @param {Object} rect1 the first rect
  * @param {Object} rect2 the second rect
  * @return {number} the height of the intersection of the rects
  */
@@ skipping 137 matching lines @@
   updateViewport_: function() {
     this.viewportChangedCallback_();
   },
 
   /**
    * @private
    * Called when the viewport size changes.
    */
   resize_: function() {
     if (this.fittingType_ == Viewport.FittingType.FIT_TO_PAGE)
-      this.fitToPage();
+      this.fitToPageAndScroll_(false);
     else if (this.fittingType_ == Viewport.FittingType.FIT_TO_WIDTH)
       this.fitToWidth();
     else
       this.updateViewport_();
   },
 
   /**
    * @type {Object} the scroll position of the viewport.
    */
   get position() {
@@ skipping 231 matching lines @@
       // document is used, which is equal to the size of the document width.
       this.setZoomInternal_(this.computeFittingZoom_(this.documentDimensions_,
                                                      true));
       var page = this.getMostVisiblePage();
       this.window_.scrollTo(0, oldY * this.zoom_);
       this.updateViewport_();
     }.bind(this));
   },
 
   /**
-   * Zoom the viewport so that a page consumes the entire viewport. Also scrolls
-   * to the top of the most visible page.
+   * @private
+   * Zoom the viewport so that a page consumes the entire viewport.
+   * @param {boolean} scrollToTopOfPage Set to true if the viewport should be
+   *     scrolled to the top of the current page. Set to false if the viewport
+   *     should remain at the current scroll position.
    */
-  fitToPage: function() {
+  fitToPageAndScroll_: function(scrollToTopOfPage) {

[Sam McNally, 2015/02/17 03:42:21]

This name suggests that it will always scroll somewhere.

[raymes, 2015/02/17 03:51:57]

Done.

     this.mightZoom_(function() {
       this.fittingType_ = Viewport.FittingType.FIT_TO_PAGE;
       if (!this.documentDimensions_)
         return;
       var page = this.getMostVisiblePage();
       // Fit to the current page's height and the widest page's width.
       var dimensions = {
         width: this.documentDimensions_.width,
         height: this.pageDimensions_[page].height,
       };
+      // Track the last y-position to stay at the same position after zooming.
+      var oldY = this.window_.pageYOffset / this.zoom_;
       this.setZoomInternal_(this.computeFittingZoom_(dimensions, false));
-      this.window_.scrollTo(0, this.pageDimensions_[page].y * this.zoom_);
+      if (scrollToTopOfPage)
+        this.window_.scrollTo(0, this.pageDimensions_[page].y * this.zoom_);
+      else
+        this.window_.scrollTo(0, oldY * this.zoom_);

[Sam McNally, 2015/02/17 03:42:21]

Shouldn't setZoomInternal_ do this?

[raymes, 2015/02/17 03:51:57]

I guess the only difference is we want to scroll to 0 x-offset. I made this
change.

       this.updateViewport_();
     }.bind(this));
   },
 
   /**
+   * Zoom the viewport so that a page consumes the entire viewport. Also scrolls
+   * the viewport to the top of the current page.
+   */
+  fitToPage: function() {
+    this.fitToPageAndScroll_(true);
+  },
+
+  /**
    * Zoom out to the next predefined zoom level.
    */
   zoomOut: function() {
     this.mightZoom_(function() {
       this.fittingType_ = Viewport.FittingType.NONE;
       var nextZoom = Viewport.ZOOM_FACTORS[0];
       for (var i = 0; i < Viewport.ZOOM_FACTORS.length; i++) {
         if (Viewport.ZOOM_FACTORS[i] < this.zoom_)
           nextZoom = Viewport.ZOOM_FACTORS[i];
       }
@@ skipping 100 matching lines @@
     spaceOnLeft = Math.max(spaceOnLeft, 0);
 
     return {
       x: x * this.zoom_ + spaceOnLeft - this.window_.pageXOffset,
       y: insetDimensions.y * this.zoom_ - this.window_.pageYOffset,
       width: insetDimensions.width * this.zoom_,
       height: insetDimensions.height * this.zoom_
     };
   }
 };