Skip to content

Commit

Permalink
Add a scroll offset to PDF documents to account for the top material …
Browse files Browse the repository at this point in the history
…design toolbar.

Previously the toolbar in the material design PDF UI would always cover the top
of pages when it was first loaded or when a page was navigated to using the
page selector. Now we ensure that a blank region is left at the very top of
the document when it is first loaded. This is the region that the toolbar
covers, so the document is not obscured at all. When pages are navigated to, we
ensure that the top of the selected page is always underneath the toolbar
so that it is not obscured. The one exception to this is when in fit-to-page
mode which causes the page to be zoomed to cover the entire screen, ignoring
the toolbar. This is so that users can take advantaging of filling all of the
screen real-estate with a page when that is what they want.

This is implemented by initially scrolling the document to a negative offset
(which is equal to the toolbar height). All subsequent scrolls are relative
to this initial scroll. A few small bugs that assumed there was no blank space
above the first page have also been fixed.

BUG=439114

Review URL: https://codereview.chromium.org/1255403002

Cr-Commit-Position: refs/heads/master@{#341685}
  • Loading branch information
raymes authored and Commit bot committed Aug 4, 2015
1 parent c515714 commit daad0f1
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 44 deletions.
16 changes: 14 additions & 2 deletions chrome/browser/resources/pdf/pdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ function shouldIgnoreKeyEvents(activeElement) {
*/
PDFViewer.MIN_TOOLBAR_OFFSET = 15;

/**
* The height of the toolbar along the top of the page. The document will be
* shifted down by this much in the viewport.
*/
PDFViewer.MATERIAL_TOOLBAR_HEIGHT = 64;

/**
* Creates a new PDFViewer. There should only be one of these objects per
* document.
Expand Down Expand Up @@ -104,13 +110,16 @@ function PDFViewer(browserApi) {
this.errorScreen_ = $('error-screen');

// Create the viewport.
var topToolbarHeight =
this.isMaterial_ ? PDFViewer.MATERIAL_TOOLBAR_HEIGHT : 0;
this.viewport_ = new Viewport(window,
this.sizer_,
this.viewportChanged_.bind(this),
this.beforeZoom_.bind(this),
this.afterZoom_.bind(this),
getScrollbarWidth(),
this.browserApi_.getDefaultZoom());
this.browserApi_.getDefaultZoom(),
topToolbarHeight);

// Create the plugin object dynamically so we can set its src. The plugin
// element is sized to fill the entire window and is set to be fixed
Expand Down Expand Up @@ -143,8 +152,11 @@ function PDFViewer(browserApi) {
}
this.plugin_.setAttribute('headers', headers);

if (this.isMaterial_)
if (this.isMaterial_) {
this.plugin_.setAttribute('is-material', '');
this.plugin_.setAttribute('top-toolbar-height',
PDFViewer.MATERIAL_TOOLBAR_HEIGHT);
}

if (!this.browserApi_.getStreamInfo().embedded)
this.plugin_.setAttribute('full-frame', '');
Expand Down
53 changes: 37 additions & 16 deletions chrome/browser/resources/pdf/viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@ function getIntersectionHeight(rect1, rect2) {
* @param {Function} afterZoomCallback is run after a change in zoom
* @param {number} scrollbarWidth the width of scrollbars on the page
* @param {number} defaultZoom The default zoom level.
* @param {number} topToolbarHeight The number of pixels that should initially
* be left blank above the document for the toolbar.
*/
function Viewport(window,
sizer,
viewportChangedCallback,
beforeZoomCallback,
afterZoomCallback,
scrollbarWidth,
defaultZoom) {
defaultZoom,
topToolbarHeight) {
this.window_ = window;
this.sizer_ = sizer;
this.viewportChangedCallback_ = viewportChangedCallback;
Expand All @@ -45,6 +48,7 @@ function Viewport(window,
this.scrollbarWidth_ = scrollbarWidth;
this.fittingType_ = Viewport.FittingType.NONE;
this.defaultZoom_ = defaultZoom;
this.topToolbarHeight_ = topToolbarHeight;

window.addEventListener('scroll', this.updateViewport_.bind(this));
window.addEventListener('resize', this.resize_.bind(this));
Expand Down Expand Up @@ -139,8 +143,8 @@ Viewport.prototype = {
if (this.documentDimensions_) {
this.sizer_.style.width =
this.documentDimensions_.width * this.zoom_ + 'px';
this.sizer_.style.height =
this.documentDimensions_.height * this.zoom_ + 'px';
this.sizer_.style.height = this.documentDimensions_.height * this.zoom_ +
this.topToolbarHeight_ + 'px';
}
},

Expand Down Expand Up @@ -171,7 +175,7 @@ Viewport.prototype = {
get position() {
return {
x: this.window_.pageXOffset,
y: this.window_.pageYOffset
y: this.window_.pageYOffset - this.topToolbarHeight_
};
},

Expand All @@ -180,7 +184,7 @@ Viewport.prototype = {
* @type {Object} position the position to scroll to.
*/
set position(position) {
this.window_.scrollTo(position.x, position.y);
this.window_.scrollTo(position.x, position.y + this.topToolbarHeight_);
},

/**
Expand Down Expand Up @@ -229,15 +233,17 @@ Viewport.prototype = {
'Viewport.mightZoom_.';
}
// Record the scroll position (relative to the top-left of the window).
var currentScrollPos = [
this.window_.pageXOffset / this.zoom_,
this.window_.pageYOffset / this.zoom_
];
var currentScrollPos = {
x: this.position.x / this.zoom_,
y: this.position.y / this.zoom_
};
this.zoom_ = newZoom;
this.contentSizeChanged_();
// Scroll to the scaled scroll position.
this.window_.scrollTo(currentScrollPos[0] * newZoom,
currentScrollPos[1] * newZoom);
this.position = {
x: currentScrollPos.x * newZoom,
y: currentScrollPos.y * newZoom
};
},

/**
Expand Down Expand Up @@ -426,8 +432,12 @@ Viewport.prototype = {
height: this.pageDimensions_[page].height,
};
this.setZoomInternal_(this.computeFittingZoom_(dimensions, false));
if (scrollToTopOfPage)
this.window_.scrollTo(0, this.pageDimensions_[page].y * this.zoom_);
if (scrollToTopOfPage) {
this.position = {
x: 0,
y: this.pageDimensions_[page].y * this.zoom_
};
}
this.updateViewport_();
}.bind(this));
},
Expand Down Expand Up @@ -485,8 +495,16 @@ Viewport.prototype = {
if (page >= this.pageDimensions_.length)
page = this.pageDimensions_.length - 1;
var dimensions = this.pageDimensions_[page];
this.window_.scrollTo(dimensions.x * this.zoom_,
dimensions.y * this.zoom_);
var toolbarOffset = 0;
// Unless we're in fit to page mode, scroll above the page by
// |this.topToolbarHeight_| so that the toolbar isn't covering it
// initially.
if (this.fittingType_ != Viewport.FittingType.FIT_TO_PAGE)
toolbarOffset = this.topToolbarHeight_;
this.position = {
x: dimensions.x * this.zoom_,
y: dimensions.y * this.zoom_ - toolbarOffset
};
this.updateViewport_();
}.bind(this));
},
Expand All @@ -504,7 +522,10 @@ Viewport.prototype = {
this.setZoomInternal_(
Math.min(this.defaultZoom_,
this.computeFittingZoom_(this.documentDimensions_, true)));
this.window_.scrollTo(0, 0);
this.position = {
x: 0,
y: -this.topToolbarHeight_
};
}
this.contentSizeChanged_();
this.resize_();
Expand Down
3 changes: 2 additions & 1 deletion chrome/test/data/pdf/basic_plugin_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ var tests = [
viewer.viewport.setZoom(1);
var sizer = document.getElementById('sizer');
chrome.test.assertEq(826, sizer.offsetWidth);
chrome.test.assertEq(1066, sizer.offsetHeight);
chrome.test.assertEq(1066 + viewer.viewport.topToolbarHeight_,
sizer.offsetHeight);
chrome.test.succeed();
},

Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/pdf/navigator_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var tests = [
var mockSizer = new MockSizer();
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);

var paramsParser = new OpenPDFParamsParser(function(name) {
if (name == 'US')
Expand Down
63 changes: 52 additions & 11 deletions chrome/test/data/pdf/viewport_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var tests = [
function testDocumentNeedsScrollbars() {
var viewport =
new Viewport(new MockWindow(100, 100), new MockSizer(), function() {},
function() {}, function() {}, 10, 1);
function() {}, function() {}, 10, 1, 0);
var scrollbars;

viewport.setDocumentDimensions(new MockDocumentDimensions(90, 90));
Expand Down Expand Up @@ -61,7 +61,7 @@ var tests = [
var mockWindow = new MockWindow(100, 100, mockSizer);
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);

// Test setting the zoom without the document dimensions set. The sizer
// shouldn't change size.
Expand Down Expand Up @@ -136,7 +136,7 @@ var tests = [
function testGetMostVisiblePage() {
var mockWindow = new MockWindow(100, 100);
var viewport = new Viewport(mockWindow, new MockSizer(), function() {},
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);

var documentDimensions = new MockDocumentDimensions(100, 100);
documentDimensions.addPage(50, 100);
Expand Down Expand Up @@ -186,7 +186,7 @@ var tests = [
var mockSizer = new MockSizer();
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);
var documentDimensions = new MockDocumentDimensions();

// Test with a document width which matches the window width.
Expand Down Expand Up @@ -245,7 +245,7 @@ var tests = [
// fit to width, which will cause the page height to span outside of the
// viewport, triggering 15px scrollbars to be shown.
viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 15, 1);
function() {}, function() {}, 15, 1, 0);
documentDimensions.reset();
documentDimensions.addPage(50, 100);
viewport.setDocumentDimensions(documentDimensions);
Expand All @@ -264,7 +264,7 @@ var tests = [
var mockSizer = new MockSizer();
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);
var documentDimensions = new MockDocumentDimensions();

// Test with a page size which matches the window size.
Expand Down Expand Up @@ -373,7 +373,7 @@ var tests = [
var mockSizer = new MockSizer();
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);
var documentDimensions = new MockDocumentDimensions();

documentDimensions.addPage(100, 100);
Expand Down Expand Up @@ -414,7 +414,7 @@ var tests = [
var mockSizer = new MockSizer();
var mockCallback = new MockViewportChangedCallback();
var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
function() {}, function() {}, 0, 1);
function() {}, function() {}, 0, 1, 0);
var documentDimensions = new MockDocumentDimensions();
documentDimensions.addPage(100, 100);
documentDimensions.addPage(200, 200);
Expand Down Expand Up @@ -469,15 +469,15 @@ var tests = [
chrome.test.assertEq(1, viewport.zoom);
};
viewport = new Viewport(mockWindow, mockSizer, function() {},
beforeZoom, afterZoom, 0, 1);
beforeZoom, afterZoom, 0, 1, 0);
viewport.setZoom(0.5);
chrome.test.succeed();
},

function testInitialSetDocumentDimensionsZoomConstrained() {
var viewport =
new Viewport(new MockWindow(100, 100), new MockSizer(), function() {},
function() {}, function() {}, 0, 1.2);
function() {}, function() {}, 0, 1.2, 0);
viewport.setDocumentDimensions(new MockDocumentDimensions(50, 50));
chrome.test.assertEq(1.2, viewport.zoom);
chrome.test.succeed();
Expand All @@ -486,11 +486,52 @@ var tests = [
function testInitialSetDocumentDimensionsZoomUnconstrained() {
var viewport = new Viewport(
new MockWindow(100, 100),
new MockSizer(), function() {}, function() {}, function() {}, 0, 3);
new MockSizer(), function() {}, function() {}, function() {}, 0, 3, 0);
viewport.setDocumentDimensions(new MockDocumentDimensions(50, 50));
chrome.test.assertEq(2, viewport.zoom);
chrome.test.succeed();
},

function testToolbarHeightOffset() {
var mockSizer = new MockSizer();
var mockWindow = new MockWindow(100, 100);
var viewport = new Viewport(mockWindow,
mockSizer, function() {}, function() {}, function() {}, 0, 1, 50);
var documentDimensions = new MockDocumentDimensions(0, 0);
documentDimensions.addPage(50, 500);
viewport.setDocumentDimensions(documentDimensions);
viewport.setZoom(1);

// Check that the sizer incorporates the toolbar height.
chrome.test.assertEq('550px', mockSizer.style.height);
chrome.test.assertEq('50px', mockSizer.style.width);
chrome.test.assertEq(0, viewport.position.x);

// Check the sizer incorporates the toolbar height correctly even if zoomed.
viewport.setZoom(2);
chrome.test.assertEq('1050px', mockSizer.style.height);
chrome.test.assertEq('100px', mockSizer.style.width);

// Test that the viewport scrolls to the correct offset when fit-to-page is
// enabled. The top of the viewport should be at the start of the document.
viewport.fitToPage();
chrome.test.assertEq(0, viewport.position.y);

// Check that going to a page scrolls to the correct offset when fit-to-page
// is enabled. The top of the viewport should be at the start of the
// document.
mockWindow.scrollTo(0, 100);
viewport.goToPage(0);
chrome.test.assertEq(0, viewport.position.y);

// Check that going to a page scrolls to the correct offset when fit-to-page
// is not enabled. The top of the viewport should be before start of the
// document.
viewport.setZoom(1);
viewport.goToPage(0);
chrome.test.assertEq(-50, viewport.position.y);
chrome.test.succeed();
}
];

chrome.test.runTests(tests);
Loading

0 comments on commit daad0f1

Please sign in to comment.