Fix zoom PDF param.
The "left" param was ignored because it was applied before the zoom,
and the page still fit in the window at 1x zoom. Scrolling on x was
non-op at this zoom level.
Bug: 319910,64309
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I7b476c018f9972f2b1e0075e933e6489f12e6974
Reviewed-on: https://chromium-review.googlesource.com/817690
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523223}diff --git a/chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js b/chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
index 15f3361..a78e9d2f 100644
--- a/chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
+++ b/chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
@@ -66,9 +66,12 @@
onClick: function() {
if (this.bookmark.hasOwnProperty('page')) {
if (this.bookmark.hasOwnProperty('y')) {
- this.fire(
- 'change-page-and-y',
- {page: this.bookmark.page, y: this.bookmark.y, origin: 'bookmark'});
+ this.fire('change-page-and-xy', {
+ page: this.bookmark.page,
+ x: 0,
+ y: this.bookmark.y,
+ origin: 'bookmark'
+ });
} else {
this.fire(
'change-page', {page: this.bookmark.page, origin: 'bookmark'});
diff --git a/chrome/browser/resources/pdf/pdf.js b/chrome/browser/resources/pdf/pdf.js
index 6e38486..5544dec 100644
--- a/chrome/browser/resources/pdf/pdf.js
+++ b/chrome/browser/resources/pdf/pdf.js
@@ -232,8 +232,8 @@
this.metrics.onPageSelectorNavigation();
});
- document.body.addEventListener('change-page-and-y', e => {
- this.viewport_.goToPageAndY(e.detail.page, e.detail.y);
+ document.body.addEventListener('change-page-and-xy', e => {
+ this.viewport_.goToPageAndXY(e.detail.page, e.detail.x, e.detail.y);
if (e.detail.origin == 'bookmark')
this.metrics.onFollowBookmark();
});
@@ -526,20 +526,16 @@
* @param {Object} params The open params passed in the URL.
*/
handleURLParams_: function(params) {
- if (params.page != undefined)
- this.viewport_.goToPage(params.page);
-
- if (params.position) {
- // Make sure we don't cancel effect of page parameter.
- this.viewport_.position = {
- x: this.viewport_.position.x + params.position.x,
- y: this.viewport_.position.y + params.position.y
- };
- }
-
if (params.zoom)
this.viewport_.setZoom(params.zoom);
+ if (params.position) {
+ this.viewport_.goToPageAndXY(
+ params.page ? params.page : 0, params.position.x, params.position.y);
+ } else if (params.page) {
+ this.viewport_.goToPage(params.page);
+ }
+
if (params.view) {
this.isUserInitiatedEvent_ = false;
this.zoomToolbar_.forceFit(params.view);
diff --git a/chrome/browser/resources/pdf/viewport.js b/chrome/browser/resources/pdf/viewport.js
index 53e1b17..709b646 100644
--- a/chrome/browser/resources/pdf/viewport.js
+++ b/chrome/browser/resources/pdf/viewport.js
@@ -798,15 +798,16 @@
* @param {number} page the index of the page to go to. zero-based.
*/
goToPage: function(page) {
- this.goToPageAndY(page, 0);
+ this.goToPageAndXY(page, 0, 0);
},
/**
* Go to the given y position in the given page index.
* @param {number} page the index of the page to go to. zero-based.
+ * @param {number} x the x position in the page to go to.
* @param {number} y the y position in the page to go to.
*/
- goToPageAndY: function(page, y) {
+ goToPageAndXY: function(page, x, y) {
this.mightZoom_(() => {
if (this.pageDimensions_.length === 0)
return;
@@ -822,7 +823,7 @@
if (!this.isPagedMode())
toolbarOffset = this.topToolbarHeight_;
this.position = {
- x: dimensions.x * this.zoom,
+ x: (dimensions.x + x) * this.zoom,
y: (dimensions.y + y) * this.zoom - toolbarOffset
};
this.updateViewport_();
diff --git a/chrome/test/data/pdf/bookmarks_test.js b/chrome/test/data/pdf/bookmarks_test.js
index cd34e1ab..b97efc7 100644
--- a/chrome/test/data/pdf/bookmarks_test.js
+++ b/chrome/test/data/pdf/bookmarks_test.js
@@ -70,37 +70,43 @@
chrome.test.assertEq(1, subBookmarks.length, "one sub bookmark");
var lastPageChange;
+ var lastXChange;
var lastYChange;
var lastUriNavigation;
bookmarkContent.addEventListener('change-page', function(e) {
lastPageChange = e.detail.page;
+ lastXChange = undefined;
lastYChange = undefined;
lastUriNavigation = undefined;
});
- bookmarkContent.addEventListener('change-page-and-y', function(e) {
+ bookmarkContent.addEventListener('change-page-and-xy', function(e) {
lastPageChange = e.detail.page;
+ lastXChange = e.detail.x;
lastYChange = e.detail.y;
lastUriNavigation = undefined;
});
bookmarkContent.addEventListener('navigate', function(e) {
lastPageChange = undefined;
+ lastXChange = undefined;
lastYChange = undefined;
lastUriNavigation = e.detail.uri;
});
function testTapTarget(tapTarget, expectedEvent) {
lastPageChange = undefined;
+ lastXChange = undefined;
lastYChange = undefined;
lastUriNavigation = undefined;
MockInteractions.tap(tapTarget);
chrome.test.assertEq(expectedEvent.page, lastPageChange);
+ chrome.test.assertEq(expectedEvent.x, lastXChange);
chrome.test.assertEq(expectedEvent.y, lastYChange);
chrome.test.assertEq(expectedEvent.uri, lastUriNavigation);
}
- testTapTarget(rootBookmarks[0].$.item, {page: 0, y: 166})
- testTapTarget(subBookmarks[0].$.item, {page: 1, y: 166})
- testTapTarget(rootBookmarks[1].$.item, {page: 2, y: 166})
+ testTapTarget(rootBookmarks[0].$.item, {page: 0, x: 0, y: 166})
+ testTapTarget(subBookmarks[0].$.item, {page: 1, x: 0, y: 166})
+ testTapTarget(rootBookmarks[1].$.item, {page: 2, x: 0, y: 166})
testTapTarget(rootBookmarks[2].$.item, {uri: "http://www.chromium.org"})
chrome.test.succeed();
diff --git a/chrome/test/data/pdf/viewport_test.js b/chrome/test/data/pdf/viewport_test.js
index 04dbbbeb..387bb10 100644
--- a/chrome/test/data/pdf/viewport_test.js
+++ b/chrome/test/data/pdf/viewport_test.js
@@ -551,6 +551,60 @@
chrome.test.succeed();
},
+ function testGoToPageAndXY() {
+ var mockWindow = new MockWindow(100, 100);
+ var mockSizer = new MockSizer();
+ var mockCallback = new MockViewportChangedCallback();
+ var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
+ function() {}, function() {}, function() {},
+ 0, 1, 0);
+ var documentDimensions = new MockDocumentDimensions();
+
+ documentDimensions.addPage(100, 100);
+ documentDimensions.addPage(200, 200);
+ documentDimensions.addPage(100, 400);
+ viewport.setDocumentDimensions(documentDimensions);
+ viewport.setZoom(1);
+
+ mockCallback.reset();
+ viewport.goToPageAndXY(0, 0, 0);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0, viewport.position.x);
+ chrome.test.assertEq(0, viewport.position.y);
+
+ mockCallback.reset();
+ viewport.goToPageAndXY(1, 0, 0);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0, viewport.position.x);
+ chrome.test.assertEq(100, viewport.position.y);
+
+ mockCallback.reset();
+ viewport.goToPageAndXY(2, 42, 46);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0 + 42, viewport.position.x);
+ chrome.test.assertEq(300 + 46, viewport.position.y);
+
+ mockCallback.reset();
+ viewport.goToPageAndXY(2, 42, 0);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0 + 42, viewport.position.x);
+ chrome.test.assertEq(300, viewport.position.y);
+
+ mockCallback.reset();
+ viewport.goToPageAndXY(2, 0, 46);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0, viewport.position.x);
+ chrome.test.assertEq(300 + 46, viewport.position.y);
+
+ viewport.setZoom(0.5);
+ mockCallback.reset();
+ viewport.goToPageAndXY(2, 42, 46);
+ chrome.test.assertTrue(mockCallback.wasCalled);
+ chrome.test.assertEq(0 + 21, viewport.position.x);
+ chrome.test.assertEq(150 + 23, viewport.position.y);
+ chrome.test.succeed();
+ },
+
function testGetPageScreenRect() {
var mockWindow = new MockWindow(100, 100);
var mockSizer = new MockSizer();
