Skip to content

GitLab

T
tangled
Commit a9c2e27e authored by dpapad's avatar dpapad Committed by Commit Bot
Browse files
Options

PDF Viewer: Make Viewport class work with HTMLElement instead of Window. 

This is necessary in order to have the Viewport class work for both
PDFViewerUpdate=false/true codepaths. Previously the Viewport
constructor was expecting a Window instance to be passed as the
scrolling container, but that is no longer the case in the
PDFViewerUpdate UI.

This CL modifies the code such that the scrolling container passed to
the Viewport constructor is:
 - the <html> element for PDFViewerUpdate=false
 - the #main element for PDFViewerUpdate=true

In order to do so
 - the <html> element is given a height of 100%, so that offsetHeight
   has a meaningful non-zero value.
 - innerHeight/innerWidth is replaced with offsetHeight/offsetWidth.
 - pageXOffset/pageYOffset is replaced with scrollLeft/scrollTop
 - a new MockElement class is introduced to replace MockWindow in some
   tests (MockWindow is still used in toolbar_manager_tests.js)

Bug: 1101598
Change-Id: I0d3fa1a8a0146c97ef636b01954911750b5be942
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308865
Commit-Queue: dpapad <dpapad@chromium.org>
Reviewed-by: default avatarRebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791686}
parent c7df96f3
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 175 additions and 74 deletions
chrome/browser/resources/pdf/index.html
View file @ a9c2e27e
...@@ -8,9 +8,12 @@ ...@@ -8,9 +8,12 @@
<link rel="stylesheet" href="index.css"> <link rel="stylesheet" href="index.css">
<style> <style>
html[pdf-viewer-update-enabled], html,
html[pdf-viewer-update-enabled] body { body {
height: 100%; height: 100%;
}
html[pdf-viewer-update-enabled] body {
width: 100%; width: 100%;
} }
</style> </style>
......
chrome/browser/resources/pdf/index_pp.html
View file @ a9c2e27e
...@@ -5,6 +5,13 @@ ...@@ -5,6 +5,13 @@
<link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css"> <link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">
<link rel="stylesheet" href="index.css"> <link rel="stylesheet" href="index.css">
<style>
html,
body {
height: 100%;
}
</style>
</head> </head>
<body> <body>
<pdf-viewer-pp id="viewer"></pdf-viewer-pp> <pdf-viewer-pp id="viewer"></pdf-viewer-pp>
......
chrome/browser/resources/pdf/pdf_viewer_base.js
View file @ a9c2e27e
...@@ -236,14 +236,23 @@ export class PDFViewerBaseElement extends PolymerElement { ...@@ -236,14 +236,23 @@ export class PDFViewerBaseElement extends PolymerElement {
}; };
} }
// Determine the scrolling container.
const pdfViewerUpdateEnabled =
document.documentElement.hasAttribute('pdf-viewer-update-enabled');
const scrollContainer = pdfViewerUpdateEnabled ?
/** @type {!HTMLElement} */ (this.getSizer().offsetParent) :
document.documentElement;
// Create the viewport. // Create the viewport.
const defaultZoom = const defaultZoom =
this.browserApi.getZoomBehavior() === BrowserApi.ZoomBehavior.MANAGE ? this.browserApi.getZoomBehavior() === BrowserApi.ZoomBehavior.MANAGE ?
this.browserApi.getDefaultZoom() : this.browserApi.getDefaultZoom() :
1.0; 1.0;
this.viewport_ = new Viewport( this.viewport_ = new Viewport(
window, this.getSizer(), this.getContent(), getScrollbarWidth(), scrollContainer, this.getSizer(), this.getContent(),
defaultZoom, this.getToolbarHeight(), this.hasFixedToolbar()); getScrollbarWidth(), defaultZoom, this.getToolbarHeight(),
this.hasFixedToolbar());
this.viewport_.setViewportChangedCallback(() => this.viewportChanged_()); this.viewport_.setViewportChangedCallback(() => this.viewportChanged_());
this.viewport_.setBeforeZoomCallback( this.viewport_.setBeforeZoomCallback(
() => this.currentController.beforeZoom()); () => this.currentController.beforeZoom());
......
chrome/browser/resources/pdf/viewport.js
View file @ a9c2e27e
...@@ -69,7 +69,7 @@ function vectorDelta(p1, p2) { ...@@ -69,7 +69,7 @@ function vectorDelta(p1, p2) {
export class Viewport { export class Viewport {
/** /**
* @param {!Window} window * @param {!HTMLElement} scrollParent
* @param {!HTMLDivElement} sizer The element which represents the size of the * @param {!HTMLDivElement} sizer The element which represents the size of the
* document in the viewport * document in the viewport
* @param {!HTMLDivElement} content The element which is the parent of the * @param {!HTMLDivElement} content The element which is the parent of the
...@@ -82,10 +82,10 @@ export class Viewport { ...@@ -82,10 +82,10 @@ export class Viewport {
* not automatically disappear in fit to page mode. * not automatically disappear in fit to page mode.
*/ */
constructor( constructor(
window, sizer, content, scrollbarWidth, defaultZoom, topToolbarHeight, scrollParent, sizer, content, scrollbarWidth, defaultZoom,
topToolbarFixed) { topToolbarHeight, topToolbarFixed) {
/** @private {!Window} */ /** @private {!HTMLElement} */
this.window_ = window; this.window_ = scrollParent;
/** @private {!HTMLDivElement} */ /** @private {!HTMLDivElement} */
this.sizer_ = sizer; this.sizer_ = sizer;
...@@ -191,8 +191,19 @@ export class Viewport { ...@@ -191,8 +191,19 @@ export class Viewport {
// Set to a default zoom manager - used in tests. // Set to a default zoom manager - used in tests.
this.setZoomManager(new InactiveZoomManager(this.getZoom.bind(this), 1)); this.setZoomManager(new InactiveZoomManager(this.getZoom.bind(this), 1));
window.addEventListener('scroll', this.updateViewport_.bind(this)); if (this.window_ === document.documentElement) {
window.addEventListener('scroll', this.updateViewport_.bind(this));
// The following line is only used in tests, since they expect
// |scrollCallback| to be called on the mock |window_| object (legacy).
this.window_.scrollCallback = this.updateViewport_.bind(this);
} else {
this.window_.addEventListener('scroll', this.updateViewport_.bind(this));
}
window.addEventListener('resize', this.resizeWrapper_.bind(this)); window.addEventListener('resize', this.resizeWrapper_.bind(this));
// The following line is only used in tests, since they expect
// |resizeCallback| to be called on the mock |window_| object (legacy).
this.window_.resizeCallback = this.resizeWrapper_.bind(this);
document.body.addEventListener( document.body.addEventListener(
'change-zoom', e => this.setZoom(e.detail.zoom)); 'change-zoom', e => this.setZoom(e.detail.zoom));
...@@ -382,15 +393,15 @@ export class Viewport { ...@@ -382,15 +393,15 @@ export class Viewport {
// If scrollbars are required for one direction, expand the document in the // If scrollbars are required for one direction, expand the document in the
// other direction to take the width of the scrollbars into account when // other direction to take the width of the scrollbars into account when
// deciding whether the other direction needs scrollbars. // deciding whether the other direction needs scrollbars.
if (zoomedDimensions.width > this.window_.innerWidth) { if (zoomedDimensions.width > this.window_.offsetWidth) {
zoomedDimensions.height += this.scrollbarWidth_; zoomedDimensions.height += this.scrollbarWidth_;
} else if (zoomedDimensions.height > this.window_.innerHeight) { } else if (zoomedDimensions.height > this.window_.offsetHeight) {
zoomedDimensions.width += this.scrollbarWidth_; zoomedDimensions.width += this.scrollbarWidth_;
} }
return { return {
horizontal: zoomedDimensions.width > this.window_.innerWidth, horizontal: zoomedDimensions.width > this.window_.offsetWidth,
vertical: zoomedDimensions.height + this.topToolbarHeight_ > vertical: zoomedDimensions.height + this.topToolbarHeight_ >
this.window_.innerHeight this.window_.offsetHeight
}; };
} }
...@@ -468,8 +479,8 @@ export class Viewport { ...@@ -468,8 +479,8 @@ export class Viewport {
/** @return {!Point} The scroll position of the viewport. */ /** @return {!Point} The scroll position of the viewport. */
get position() { get position() {
return { return {
x: this.window_.pageXOffset, x: this.window_.scrollLeft,
y: this.window_.pageYOffset - this.topToolbarHeight_ y: this.window_.scrollTop - this.topToolbarHeight_
}; };
} }
...@@ -488,8 +499,8 @@ export class Viewport { ...@@ -488,8 +499,8 @@ export class Viewport {
const scrollbarHeight = const scrollbarHeight =
needsScrollbars.horizontal ? this.scrollbarWidth_ : 0; needsScrollbars.horizontal ? this.scrollbarWidth_ : 0;
return { return {
width: this.window_.innerWidth - scrollbarWidth, width: this.window_.offsetWidth - scrollbarWidth,
height: this.window_.innerHeight - scrollbarHeight height: this.window_.offsetHeight - scrollbarHeight
}; };
} }
...@@ -804,12 +815,12 @@ export class Viewport { ...@@ -804,12 +815,12 @@ export class Viewport {
'true.'); 'true.');
// First compute the zoom without scrollbars. // First compute the zoom without scrollbars.
let height = this.window_.innerHeight; let height = this.window_.offsetHeight;
if (this.topToolbarFixed_) { if (this.topToolbarFixed_) {
height -= this.topToolbarHeight_; height -= this.topToolbarHeight_;
} }
let zoom = this.computeFittingZoomGivenDimensions_( let zoom = this.computeFittingZoomGivenDimensions_(
fitWidth, fitHeight, this.window_.innerWidth, height, fitWidth, fitHeight, this.window_.offsetWidth, height,
pageDimensions.width, pageDimensions.height); pageDimensions.width, pageDimensions.height);
// Check if there needs to be any scrollbars. // Check if there needs to be any scrollbars.
...@@ -825,17 +836,17 @@ export class Viewport { ...@@ -825,17 +836,17 @@ export class Viewport {
// Check if adding a scrollbar will result in needing the other scrollbar. // Check if adding a scrollbar will result in needing the other scrollbar.
const scrollbarWidth = this.scrollbarWidth_; const scrollbarWidth = this.scrollbarWidth_;
if (needsScrollbars.horizontal && if (needsScrollbars.horizontal &&
zoomedDimensions.height > this.window_.innerHeight - scrollbarWidth) { zoomedDimensions.height > this.window_.offsetHeight - scrollbarWidth) {
needsScrollbars.vertical = true; needsScrollbars.vertical = true;
} }
if (needsScrollbars.vertical && if (needsScrollbars.vertical &&
zoomedDimensions.width > this.window_.innerWidth - scrollbarWidth) { zoomedDimensions.width > this.window_.offsetWidth - scrollbarWidth) {
needsScrollbars.horizontal = true; needsScrollbars.horizontal = true;
} }
// Compute available window space. // Compute available window space.
const windowWithScrollbars = { const windowWithScrollbars = {
width: this.window_.innerWidth, width: this.window_.offsetWidth,
height: height, height: height,
}; };
if (needsScrollbars.horizontal) { if (needsScrollbars.horizontal) {
...@@ -1309,8 +1320,8 @@ export class Viewport { ...@@ -1309,8 +1320,8 @@ export class Viewport {
spaceOnLeft = Math.max(spaceOnLeft, 0); spaceOnLeft = Math.max(spaceOnLeft, 0);
return { return {
x: x * zoom + spaceOnLeft - this.window_.pageXOffset, x: x * zoom + spaceOnLeft - this.window_.scrollLeft,
y: insetDimensions.y * zoom - this.window_.pageYOffset, y: insetDimensions.y * zoom - this.window_.scrollTop,
width: insetDimensions.width * zoom, width: insetDimensions.width * zoom,
height: insetDimensions.height * zoom height: insetDimensions.height * zoom
}; };
...@@ -1395,7 +1406,7 @@ export class Viewport { ...@@ -1395,7 +1406,7 @@ export class Viewport {
} }
this.sentPinchEvent_ = true; this.sentPinchEvent_ = true;
this.window_.requestAnimationFrame(() => { window.requestAnimationFrame(() => {
this.sentPinchEvent_ = false; this.sentPinchEvent_ = false;
this.mightZoom_(() => { this.mightZoom_(() => {
const {direction, center, startScaleRatio} = e.detail; const {direction, center, startScaleRatio} = e.detail;
...@@ -1423,8 +1434,8 @@ export class Viewport { ...@@ -1423,8 +1434,8 @@ export class Viewport {
// using the gesture center. // using the gesture center.
if (!needsScrollbars.horizontal) { if (!needsScrollbars.horizontal) {
this.pinchCenter_ = { this.pinchCenter_ = {
x: this.window_.innerWidth / 2, x: this.window_.offsetWidth / 2,
y: this.window_.innerHeight / 2 y: this.window_.offsetHeight / 2
}; };
} else if (this.keepContentCentered_) { } else if (this.keepContentCentered_) {
this.oldCenterInContent_ = this.oldCenterInContent_ =
...@@ -1448,7 +1459,7 @@ export class Viewport { ...@@ -1448,7 +1459,7 @@ export class Viewport {
onPinchEnd_(e) { onPinchEnd_(e) {
// Using rAF for pinch end prevents pinch updates scheduled by rAF getting // Using rAF for pinch end prevents pinch updates scheduled by rAF getting
// sent after the pinch end. // sent after the pinch end.
this.window_.requestAnimationFrame(() => { window.requestAnimationFrame(() => {
this.mightZoom_(() => { this.mightZoom_(() => {
const {center, startScaleRatio} = e.detail; const {center, startScaleRatio} = e.detail;
this.pinchPhase_ = Viewport.PinchPhase.PINCH_END; this.pinchPhase_ = Viewport.PinchPhase.PINCH_END;
...@@ -1475,7 +1486,7 @@ export class Viewport { ...@@ -1475,7 +1486,7 @@ export class Viewport {
onPinchStart_(e) { onPinchStart_(e) {
// We also use rAF for pinch start, so that if there is a pinch end event // We also use rAF for pinch start, so that if there is a pinch end event
// scheduled by rAF, this pinch start will be sent after. // scheduled by rAF, this pinch start will be sent after.
this.window_.requestAnimationFrame(() => { window.requestAnimationFrame(() => {
this.pinchPhase_ = Viewport.PinchPhase.PINCH_START; this.pinchPhase_ = Viewport.PinchPhase.PINCH_START;
this.prevScale_ = 1; this.prevScale_ = 1;
this.oldCenterInContent_ = this.oldCenterInContent_ =
......
chrome/test/data/pdf/navigator_test.js
View file @ a9c2e27e
...@@ -6,7 +6,7 @@ import {PdfNavigator} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/ ...@@ -6,7 +6,7 @@ import {PdfNavigator} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/
import {OpenPdfParamsParser} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/open_pdf_params_parser.js'; import {OpenPdfParamsParser} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/open_pdf_params_parser.js';
import {PDFScriptingAPI} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_scripting_api.js'; import {PDFScriptingAPI} from 'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_scripting_api.js';
import {getZoomableViewport, MockDocumentDimensions, MockSizer, MockViewportChangedCallback, MockWindow} from './test_util.js'; import {getZoomableViewport, MockDocumentDimensions, MockElement, MockSizer, MockViewportChangedCallback} from './test_util.js';
class MockNavigatorDelegate { class MockNavigatorDelegate {
constructor() { constructor() {
...@@ -78,7 +78,7 @@ function doNavigationUrlTest( ...@@ -78,7 +78,7 @@ function doNavigationUrlTest(
* tab, and a new window. * tab, and a new window.
*/ */
function doNavigationUrlTests(originalUrl, url, expectedResultUrl) { function doNavigationUrlTests(originalUrl, url, expectedResultUrl) {
const mockWindow = new MockWindow(100, 100); const mockWindow = new MockElement(100, 100);
const mockSizer = new MockSizer(); const mockSizer = new MockSizer();
const mockViewportChangedCallback = new MockViewportChangedCallback(); const mockViewportChangedCallback = new MockViewportChangedCallback();
const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0); const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0);
...@@ -109,7 +109,7 @@ const tests = [ ...@@ -109,7 +109,7 @@ const tests = [
* opening a url in a new tab. * opening a url in a new tab.
*/ */
function testNavigate() { function testNavigate() {
const mockWindow = new MockWindow(100, 100); const mockWindow = new MockElement(100, 100);
const mockSizer = new MockSizer(); const mockSizer = new MockSizer();
const mockCallback = new MockViewportChangedCallback(); const mockCallback = new MockViewportChangedCallback();
const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0); const viewport = getZoomableViewport(mockWindow, mockSizer, 0, 1, 0);
......
chrome/test/data/pdf/test_util.js
View file @ a9c2e27e
...@@ -102,6 +102,76 @@ export class MockWindow { ...@@ -102,6 +102,76 @@ export class MockWindow {
} }
} }
export class MockElement {
/**
* @param {number} width
* @param {number} height
* @param {?HTMLDivElement} sizer
*/
constructor(width, height, sizer) {
/** @type {number} */
this.offsetWidth = width;
/** @type {number} */
this.offsetHeight = height;
/** @type {?Element} */
this.sizer = sizer;
if (sizer) {
sizer.resizeCallback_ = () =>
this.scrollTo(this.scrollLeft, this.scrollTop);
}
/** @type {number} */
this.scrollLeft = 0;
/** @type {number} */
this.scrollTop = 0;
/** @type {?Function} */
this.scrollCallback = null;
/** @type {?Function} */
this.resizeCallback = null;
}
/**
* @param {string} e The event name
* @param {!Function} f The callback
*/
addEventListener(e, f) {
if (e === 'scroll') {
this.scrollCallback = f;
}
}
/**
* @param {number} width
* @param {number} height
*/
setSize(width, height) {
this.offsetWidth = width;
this.offsetHeight = height;
this.resizeCallback();
}
/**
* @param {number} x
* @param {number} y
*/
scrollTo(x, y) {
if (this.sizer) {
x = Math.min(x, parseInt(this.sizer.style.width, 10) - this.offsetWidth);
y = Math.min(
y, parseInt(this.sizer.style.height, 10) - this.offsetHeight);
}
this.scrollLeft = Math.max(0, x);
this.scrollTop = Math.max(0, y);
this.scrollCallback();
}
}
export class MockSizer { export class MockSizer {
constructor() { constructor() {
const sizer = this; const sizer = this;
...@@ -226,7 +296,7 @@ export function createBookmarksForTest() { ...@@ -226,7 +296,7 @@ export function createBookmarksForTest() {
/** /**
* Create a viewport with basic default zoom values. * Create a viewport with basic default zoom values.
* @param {!Window} window * @param {!HTMLElement} scrollParent
* @param {!Element} sizer The element which represents the size of the * @param {!Element} sizer The element which represents the size of the
* document in the viewport * document in the viewport
* @param {number} scrollbarWidth The width of scrollbars on the page * @param {number} scrollbarWidth The width of scrollbars on the page
...@@ -236,12 +306,13 @@ export function createBookmarksForTest() { ...@@ -236,12 +306,13 @@ export function createBookmarksForTest() {
* @return {!Viewport} The viewport object with zoom values set. * @return {!Viewport} The viewport object with zoom values set.
*/ */
export function getZoomableViewport( export function getZoomableViewport(
window, sizer, scrollbarWidth, defaultZoom, topToolbarHeight) { scrollParent, sizer, scrollbarWidth, defaultZoom, topToolbarHeight) {
document.body.innerHTML = '';
const dummyContent = const dummyContent =
/** @type {!HTMLDivElement} */ (document.createElement('div')); /** @type {!HTMLDivElement} */ (document.createElement('div'));
document.body.appendChild(dummyContent); document.body.appendChild(dummyContent);
const viewport = new Viewport( const viewport = new Viewport(
window, /** @type {!HTMLDivElement} */ (sizer), dummyContent, scrollParent, /** @type {!HTMLDivElement} */ (sizer), dummyContent,
scrollbarWidth, defaultZoom, topToolbarHeight, false); scrollbarWidth, defaultZoom, topToolbarHeight, false);
viewport.setZoomFactorRange([0.25, 0.4, 0.5, 1, 2]); viewport.setZoomFactorRange([0.25, 0.4, 0.5, 1, 2]);
return viewport; return viewport;
......
chrome/test/data/pdf/viewport_test.js
View file @ a9c2e27e
This diff is collapsed.
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