Commit 425cd7d0 authored by rbpotter's avatar rbpotter Committed by Commit Bot

Print Preview: Prevent empty grey area in plugin

When the user changes focus between custom margins inputs, this causes
the preview area to scroll, which should not occur since the iframe
containing the plugin is in a fixed location. Work around this issue
in the same way as in the old UI: manually set the offsets of the
preview area element to 0.

This workaround has the downside that the newly focused input is not
visible to the user, and therefore key events cause another scroll,
which is not addressed by the workaround. To address this issue, after
setting the offsets, inform the plugin that it should scroll to make
the input visible.

Bug: 901777
Change-Id: If310b84229fd53ffaee449699b8ac8a13806101c
Reviewed-on: https://chromium-review.googlesource.com/c/1351950Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612506}
parent 82dab725
......@@ -236,6 +236,14 @@ PDFScriptingAPI.prototype = {
this.sendMessage_(
{type: 'sendKeyEvent', keyEvent: SerializeKeyEvent(keyEvent)});
},
/**
* @param {number} scrollX The amount to horizontally scroll in pixels.
* @param {number} scrollY The amount to vertically scroll in pixels.
*/
scrollPosition: function(scrollX, scrollY) {
this.sendMessage_({type: 'scrollPosition', x: scrollX, y: scrollY});
},
};
/**
......@@ -268,5 +276,6 @@ function PDFCreateOutOfProcessPlugin(src, baseUrl) {
iframe.resetPrintPreviewMode = client.resetPrintPreviewMode.bind(client);
iframe.loadPreviewPage = client.loadPreviewPage.bind(client);
iframe.sendKeyEvent = client.sendKeyEvent.bind(client);
iframe.scrollPosition = client.scrollPosition.bind(client);
return iframe;
}
......@@ -1084,6 +1084,12 @@ PDFViewer.prototype = {
case 'sendKeyEvent':
this.handleKeyEvent_(DeserializeKeyEvent(message.data.keyEvent));
return true;
case 'scrollPosition':
var position = this.viewport_.position;
position.y += message.data.y;
position.x += message.data.x;
this.viewport.position = position;
return true;
}
return false;
......
......@@ -89,7 +89,7 @@
<div id="line"></div>
<cr-input id="textbox" aria-hidden$="[[getAriaHidden_(invisible)]]"
type="text" disabled="[[invisible]]" on-blur="onBlur_"
invalid="[[invalid]]" data-timeout-delay="1000">
on-focus="onFocus_" invalid="[[invalid]]" data-timeout-delay="1000">
</cr-input>
</template>
<script src="margin_control.js"></script>
......
......@@ -149,8 +149,12 @@ Polymer({
/** @private */
onBlur_: function() {
this.resetAndUpdate();
if (this.invalid)
this.fire('text-blur');
this.fire('text-blur', this.invalid);
},
/** @private */
onFocus_: function() {
this.fire('text-focus');
},
/** @private */
......
......@@ -30,7 +30,7 @@
clip-size="[[clipSize_]]" scale-transform="[[scaleTransform_]]"
page-size="[[pageSize]]" on-pointerdown="onPointerDown_"
on-text-change="onTextChange_" on-text-blur="onTextBlur_"
on-transition-end="onTransitionEnd_">
on-text-focus="onTextFocus_" on-transition-end="onTransitionEnd_">
</print-preview-margin-control>
</template>
</template>
......
......@@ -113,6 +113,9 @@ Polymer({
},
},
/** @private {boolean} */
textboxFocused_: false,
observers: [
'onMarginSettingsChange_(settings.customMargins.value)',
'onMediaSizeOrLayoutChange_(' +
......@@ -321,13 +324,65 @@ Polymer({
if (!this.available_)
return;
// Do not set the controls invisible if the user is dragging one of them.
if (invisible && this.dragging_ != '')
// Do not set the controls invisible if the user is dragging or focusing
// the textbox for one of them.
if (invisible && (this.dragging_ != '' || this.textboxFocused_))
return;
this.invisible_ = invisible;
},
/**
* @param {!CustomEvent} e Contains information about what control fired the
* event.
* @private
*/
onTextFocus_: function(e) {
this.textboxFocused_ = true;
const control = /** @type {!PrintPreviewMarginControlElement} */ (e.target);
const x = control.offsetLeft;
const y = control.offsetTop;
const isTopOrBottom = this.isTopOrBottom_(
/** @type {!print_preview.ticket_items.CustomMarginsOrientation} */ (
control.side));
const position = {};
// Extra padding, in px, to ensure the full textbox will be visible and not
// just a portion of it. Can't be less than half the width or height of the
// clip area for the computations below to work.
const padding = Math.min(
Math.min(this.clipSize_.width / 2, this.clipSize_.height / 2), 50);
// Note: clipSize_ gives the current visible area of the margin control
// container. The offsets of the controls are relative to the origin of this
// visible area.
if (isTopOrBottom) {
// For top and bottom controls, the horizontal position of the box is
// around halfway across the control's width.
position.x = Math.min(x + control.offsetWidth / 2 - padding, 0);
position.x = Math.max(
x + control.offsetWidth / 2 + padding - this.clipSize_.width,
position.x);
// For top and bottom controls, the vertical position of the box is nearly
// the same as the vertical position of the control.
position.y = Math.min(y - padding, 0);
position.y = Math.max(y - this.clipSize_.height + padding, position.y);
} else {
// For left and right controls, the horizontal position of the box is
// nearly the same as the horizontal position of the control.
position.x = Math.min(x - padding, 0);
position.x = Math.max(x - this.clipSize_.width + padding, position.x);
// For top and bottom controls, the vertical position of the box is
// around halfway up the control's height.
position.y = Math.min(y + control.offsetHeight / 2 - padding, 0);
position.y = Math.max(
y + control.offsetHeight / 2 + padding - this.clipSize_.height,
position.y);
}
this.fire('text-focus-position', position);
},
/**
* @param {string} side The margin side. Must be a CustomMarginsOrientation.
* @param {number} marginValue New value for the margin in points.
......@@ -394,16 +449,19 @@ Polymer({
},
/**
* @param {!CustomEvent} e Event fired when a control with an invalid value's
* text field is blurred.
* @param {!CustomEvent} e Event fired when a control's text field is blurred.
* Contains information about whether the control is in an invalid state.
* @private
*/
onTextBlur_: function(e) {
const control =
/** @type {!PrintPreviewMarginControlElement} */ (e.target);
control.setTextboxValue(
this.serializeValueFromPts_(control.getPositionInPts()));
control.invalid = false;
if (e.detail /* detail is true if the control is in an invalid state */) {
const control =
/** @type {!PrintPreviewMarginControlElement} */ (e.target);
control.setTextboxValue(
this.serializeValueFromPts_(control.getPositionInPts()));
control.invalid = false;
}
this.textboxFocused_ = false;
},
/**
......
......@@ -124,6 +124,14 @@ cr.define('print_preview_new', function() {
this.getPreviewUrl_(previewUid, index), color, pages, modifiable);
}
/**
* @param {number} scrollX The amount to horizontally scroll in pixels.
* @param {number} scrollY The amount to vertically scroll in pixels.
*/
scrollPosition(scrollX, scrollY) {
this.plugin_.scrollPosition(scrollX, scrollY);
}
/** @param {!KeyboardEvent} e Keyboard event to forward to the plugin. */
sendKeyEvent(e) {
this.plugin_.sendKeyEvent(e);
......
......@@ -136,6 +136,7 @@
document-margins="[[documentInfo.margins]]"
measurement-system="[[measurementSystem]]" state="[[state]]"
preview-loaded="[[previewLoaded_]]"
on-text-focus-position="onTextFocusPosition_"
on-margin-drag-changed="onMarginDragChanged_">
</print-preview-margin-control-container>
</template>
......
......@@ -553,6 +553,28 @@ Polymer({
this.pluginProxy_.setPointerEvents(!e.detail);
},
/**
* @param {!CustomEvent} e Contains information about where the plugin
* should scroll to.
* @private
*/
onTextFocusPosition_: function(e) {
// TODO(tkent): This is a workaround of a preview-area scrolling
// issue. Blink scrolls preview-area on focus, but we don't want it. We
// should adjust scroll position of PDF preview and positions of
// MarginContgrols here, or restructure the HTML so that the PDF review
// and MarginControls are on the single scrollable container.
// crbug.com/601341
this.scrollTop = 0;
this.scrollLeft = 0;
const position = /** @type {{ x: number, y: number }} */ (e.detail);
if (position.x === 0 && position.y === 0)
return;
this.pluginProxy_.scrollPosition(position.x, position.y);
},
/** @private */
onMarginsChanged_: function() {
if (this.getSettingValue('margins') !=
......
......@@ -14,6 +14,8 @@ cr.define('custom_margins_test', function() {
LayoutClearsCustomMargins: 'layout clears custom margins',
IgnoreDocumentMarginsFromPDF: 'ignore document margins from pdf',
MediaSizeClearsCustomMarginsPDF: 'media size clears custom margins pdf',
RequestScrollToOutOfBoundsTextbox:
'request scroll to out of bounds textbox',
};
const suiteName = 'CustomMarginsTest';
......@@ -544,6 +546,64 @@ cr.define('custom_margins_test', function() {
'mediaSize', {height_microns: 200000, width_microns: 200000});
});
function whenAnimationFrameDone() {
return new Promise(resolve => window.requestAnimationFrame(resolve));
}
// Test that if the user focuses a textbox that is not visible, the
// text-focus event is fired with the correct values to scroll by.
test(assert(TestNames.RequestScrollToOutOfBoundsTextbox), function() {
return finishSetup()
.then(() => {
// Wait for the controls to be set up, which occurs in an
// animation frame.
return whenAnimationFrameDone();
})
.then(() => {
const onTransitionEnd = getAllTransitions(getControls());
// Controls become visible when margin type CUSTOM is selected.
container.set(
'settings.margins.value',
print_preview.ticket_items.MarginsTypeValue.CUSTOM);
container.notifyPath('settings.customMargins.value');
Polymer.dom.flush();
return onTransitionEnd;
})
.then(() => {
// Zoom in by 2x, so that some margin controls will not be visible.
container.updateScaleTransform(pixelsPerInch * 2 / pointsPerInch);
Polymer.dom.flush();
return whenAnimationFrameDone();
})
.then(() => {
const controls = getControls();
assertEquals(4, controls.length);
// Focus the bottom control, which is currently not visible since
// the viewer is showing only the top left quarter of the page.
const bottomControl = controls[2];
const whenEventFired =
test_util.eventToPromise('text-focus-position', container);
bottomControl.$.textbox.focus();
// Workaround for mac so that this does not need to be an
// interactive test: manually fire the focus event from the control.
bottomControl.fire('text-focus');
return whenEventFired;
})
.then((args) => {
// Shifts left by padding of 50px to ensure that the full textbox is
// visible.
assertEquals(50, args.detail.x);
// Offset top will be 2097 = 200 px/in / 72 pts/in * (794pts -
// 36ptx) - 9px radius of line
// Height of the clip box is 200 px/in * 11in = 2200px
// Shifts down by offsetTop = 2097 - height / 2 + padding = 1047px.
// This will ensure that the textbox is in the visible area.
assertEquals(1047, args.detail.y);
});
});
});
return {
......
......@@ -836,6 +836,13 @@ TEST_F(
custom_margins_test.TestNames.MediaSizeClearsCustomMarginsPDF);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'RequestScrollToOutOfBoundsTextbox',
function() {
this.runMochaTest(
custom_margins_test.TestNames.RequestScrollToOutOfBoundsTextbox);
});
PrintPreviewNewDestinationSearchTest = class extends NewPrintPreviewTest {
/** @override */
get browsePreload() {
......
......@@ -60,6 +60,12 @@ cr.define('print_preview', function() {
*/
resetPrintPreviewMode(previewUid, index, color, pages, modifiable) {}
/**
* @param {number} scrollX The amount to horizontally scroll in pixels.
* @param {number} scrollY The amount to vertically scroll in pixels.
*/
scrollPosition(scrollX, scrollY) {}
/** @param {!KeyEvent} e Keyboard event to forward to the plugin. */
sendKeyEvent(e) {}
......
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