PDF Viewer: Confirm zoom value only on enter/blur (M87)
Bug: 1136703
Change-Id: If1370e61db35049f12e22beb6219651056b326ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461801
Commit-Queue: Rebekah Potter <[email protected]>
Reviewed-by: dpapad <[email protected]>
Cr-Commit-Position: refs/heads/master@{#815768}
(cherry picked from commit e7afbcecb7674c6a55876ef45c42ec1e0d895a74)
[email protected]
Change-Id: I7910aa8f92aab6d8b49f1f56ee030625d587d14d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468298
Reviewed-by: Rebekah Potter <[email protected]>
Reviewed-by: dpapad <[email protected]>
Commit-Queue: Rebekah Potter <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#325}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.html b/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.html
index 055a631..4d77777 100644
--- a/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.html
+++ b/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.html
@@ -190,8 +190,8 @@
</cr-icon-button>
<input type="text" value="100%"
aria-label="$i18n{zoomTextInputAriaLabel}"
- on-input="onZoomInput_" on-pointerup="onZoomInputPointerup_"
- on-blur="onZoomInputBlur_">
+ on-change="onZoomChange_" on-pointerup="onZoomInputPointerup_"
+ on-blur="onZoomChange_">
</input>
<cr-icon-button iron-icon="pdf:add" title="$i18n{tooltipZoomIn}"
aria-label="$i18n{tooltipZoomIn}" on-click="onZoomInClick_">
diff --git a/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.js b/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.js
index 5acfccd..30dc170 100644
--- a/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.js
+++ b/chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.js
@@ -76,6 +76,8 @@
type: Number,
observer: 'viewportZoomChanged_',
},
+ /** @type {!{min: number, max: number}} */
+ zoomBounds: Object,
sidenavCollapsed: Boolean,
twoUpViewEnabled: Boolean,
@@ -127,9 +129,6 @@
/** @private {boolean} */
this.moreMenuOpen_ = false;
-
- /** @private {?number} */
- this.zoomTimeout_ = null;
}
/**
@@ -293,20 +292,25 @@
}
/** @private */
- onZoomInput_() {
- if (this.zoomTimeout_) {
- clearTimeout(this.zoomTimeout_);
+ onZoomChange_() {
+ const input = this.getZoomInput_();
+ let value = Number.parseInt(input.value, 10);
+ value = Math.max(Math.min(value, this.zoomBounds.max), this.zoomBounds.min);
+ if (this.sendZoomChanged_(value)) {
+ return;
}
- this.zoomTimeout_ = setTimeout(() => this.sendZoomChanged_(), 250);
+
+ const zoom = Math.round(this.viewportZoom * 100);
+ const zoomString = `${zoom}%`;
+ input.value = zoomString;
}
/**
+ * @param {number} value The new zoom value
* @return {boolean} Whether the zoom-changed event was sent.
* @private
*/
- sendZoomChanged_() {
- this.zoomTimeout_ = null;
- const value = Number.parseInt(this.getZoomInput_().value, 10);
+ sendZoomChanged_(value) {
if (Number.isNaN(value)) {
return false;
}
@@ -320,21 +324,6 @@
return true;
}
- /** @private */
- onZoomInputBlur_() {
- if (this.zoomTimeout_) {
- clearTimeout(this.zoomTimeout_);
- }
-
- if (this.sendZoomChanged_()) {
- return;
- }
-
- const zoom = Math.round(this.viewportZoom * 100);
- const zoomString = `${zoom}%`;
- this.getZoomInput_().value = zoomString;
- }
-
/**
* @param {!Event} e
* @private
diff --git a/chrome/browser/resources/pdf/pdf_viewer.html b/chrome/browser/resources/pdf/pdf_viewer.html
index bc4cdd4a..775c73e 100644
--- a/chrome/browser/resources/pdf/pdf_viewer.html
+++ b/chrome/browser/resources/pdf/pdf_viewer.html
@@ -161,7 +161,7 @@
is-form-field-focused="[[isFormFieldFocused_]]"
sidenav-collapsed="[[sidenavCollapsed_]]"
two-up-view-enabled="[[twoUpViewEnabled_]]"
- viewport-zoom="[[viewportZoom_]]"
+ viewport-zoom="[[viewportZoom_]]" zoom-bounds="[[zoomBounds_]]"
<if expr="chromeos">
annotation-available="[[annotationAvailable_]]"
ink-controller="[[inkController_]]"
diff --git a/chrome/browser/resources/pdf/pdf_viewer.js b/chrome/browser/resources/pdf/pdf_viewer.js
index 554f88b..2a2decfb 100644
--- a/chrome/browser/resources/pdf/pdf_viewer.js
+++ b/chrome/browser/resources/pdf/pdf_viewer.js
@@ -169,6 +169,7 @@
pdfAnnotationsEnabled_: Boolean,
printingEnabled_: Boolean,
viewportZoom_: Number,
+ zoomBounds_: Object,
};
}
@@ -232,6 +233,9 @@
/** @private {number} */
this.viewportZoom_ = 1;
+ /** @private {!{ min: number, max: number }} */
+ this.zoomBounds_ = {min: 0, max: 0};
+
// Non-Polymer properties
/** @type {number} */
@@ -751,6 +755,10 @@
loadTimeData.getBoolean('pdfAnnotationsEnabled');
this.pdfFormSaveEnabled_ = loadTimeData.getBoolean('pdfFormSaveEnabled');
this.printingEnabled_ = loadTimeData.getBoolean('printingEnabled');
+ const presetZoomFactors = this.viewport.presetZoomFactors;
+ this.zoomBounds_.min = Math.round(presetZoomFactors[0] * 100);
+ this.zoomBounds_.max =
+ Math.round(presetZoomFactors[presetZoomFactors.length - 1] * 100);
}
/** @override */
diff --git a/chrome/browser/resources/pdf/viewport.js b/chrome/browser/resources/pdf/viewport.js
index 11ef512b..c27b902 100644
--- a/chrome/browser/resources/pdf/viewport.js
+++ b/chrome/browser/resources/pdf/viewport.js
@@ -501,6 +501,11 @@
return this.zoomManager_.applyBrowserZoom(this.internalZoom_);
}
+ /** @return {!Array<number>} The preset zoom factors. */
+ get presetZoomFactors() {
+ return this.presetZoomFactors_;
+ }
+
/** @param {!ZoomManager} manager */
setZoomManager(manager) {
this.resetTracker();
diff --git a/chrome/test/data/pdf/viewer_pdf_toolbar_new_test.js b/chrome/test/data/pdf/viewer_pdf_toolbar_new_test.js
index 4364845..7a028b8 100644
--- a/chrome/test/data/pdf/viewer_pdf_toolbar_new_test.js
+++ b/chrome/test/data/pdf/viewer_pdf_toolbar_new_test.js
@@ -141,6 +141,7 @@
function testZoomField() {
const toolbar = createToolbar();
toolbar.viewportZoom = .8;
+ toolbar.zoomBounds = {min: 25, max: 500};
const zoomField = toolbar.shadowRoot.querySelector('#zoom-controls input');
chrome.test.assertEq('80%', zoomField.value);
@@ -148,15 +149,50 @@
toolbar.viewportZoom = .533;
chrome.test.assertEq('53%', zoomField.value);
- // Setting a new value sends the value in a zoom-changed event.
- let sentValue = -1;
- toolbar.addEventListener('zoom-changed', e => {
- sentValue = e.detail;
- chrome.test.assertEq(110, sentValue);
- chrome.test.succeed();
- });
- zoomField.value = '110%';
- zoomField.dispatchEvent(new CustomEvent('input'));
+ // Setting a non-number value resets to viewport zoom.
+ zoomField.value = 'abc';
+ zoomField.dispatchEvent(new CustomEvent('change'));
+ chrome.test.assertEq('53%', zoomField.value);
+
+ // Setting a value that is over the max zoom clips to the max value.
+ const whenSent = eventToPromise('zoom-changed', toolbar);
+ zoomField.value = '90000%';
+ zoomField.dispatchEvent(new CustomEvent('change'));
+ whenSent
+ .then(e => {
+ chrome.test.assertEq(500, e.detail);
+
+ // This happens in the parent.
+ toolbar.viewportZoom = 5;
+ chrome.test.assertEq('500%', zoomField.value);
+
+ // Setting a value that is over the maximum again restores the max
+ // value, even though no event is sent.
+ zoomField.value = '80000%';
+ zoomField.dispatchEvent(new CustomEvent('change'));
+ chrome.test.assertEq('500%', zoomField.value);
+
+ // Setting a new value sends the value in a zoom-changed event.
+ const whenSentNew = eventToPromise('zoom-changed', toolbar);
+ zoomField.value = '110%';
+ zoomField.dispatchEvent(new CustomEvent('change'));
+ return whenSentNew;
+ })
+ .then(e => {
+ chrome.test.assertEq(110, e.detail);
+
+ // Setting a new value and blurring sends the value in a zoom-changed
+ // event. If the value is below the minimum, this sends the minimum
+ // zoom.
+ const whenSentFromBlur = eventToPromise('zoom-changed', toolbar);
+ zoomField.value = '18%';
+ zoomField.dispatchEvent(new CustomEvent('blur'));
+ return whenSentFromBlur;
+ })
+ .then(e => {
+ chrome.test.assertEq(25, e.detail);
+ chrome.test.succeed();
+ });
},
// Test that the overflow menu closes when an action is triggered.
