Commit 0a1022f0 authored by rbpotter's avatar rbpotter Committed by Commit Bot

PDF Viewer: Fix edge case at bottom of document

If more than one page was visible in the viewport, getPageAtY_(y) fell
through to returning 0 when y was the bottom of the document, because
in the last iteration of the loop, where max = min = page = N-1 (for an
N page document), the bottom of the page was exactly equal to y, rather
than strictly > y.

This resulted in the most visible page being set to N-1 rather than N
when the document was scrolled to the bottom in this case.

Bug: 1000121
Change-Id: I541204141d1fb7891f1df1bc95d00be28d39bbfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1851086
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704717}
parent 9806cf54
...@@ -621,7 +621,7 @@ class Viewport { ...@@ -621,7 +621,7 @@ class Viewport {
const bottom = const bottom =
this.pageDimensions_[page].y + this.pageDimensions_[page].height; this.pageDimensions_[page].y + this.pageDimensions_[page].height;
if (top <= y && bottom > y) { if (top <= y && y <= bottom) {
return page; return page;
} }
......
...@@ -221,6 +221,11 @@ var tests = [ ...@@ -221,6 +221,11 @@ var tests = [
mockWindow.scrollTo(0, 170); mockWindow.scrollTo(0, 170);
chrome.test.assertEq(1, viewport.getMostVisiblePage()); chrome.test.assertEq(1, viewport.getMostVisiblePage());
// Zoom out so that more than one page fits and scroll to the bottom.
viewport.setZoom(0.4);
mockWindow.scrollTo(0, 160);
chrome.test.assertEq(2, viewport.getMostVisiblePage());
// Zoomed out with the entire document visible. // Zoomed out with the entire document visible.
viewport.setZoom(0.25); viewport.setZoom(0.25);
mockWindow.scrollTo(0, 0); mockWindow.scrollTo(0, 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment