PDF Viewer: Confirm zoom value only on enter/blur

Bug: 1136703
Change-Id: If1370e61db35049f12e22beb6219651056b326ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461801
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815768}

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_">

chrome/browser/resources/pdf/elements/viewer-pdf-toolbar-new.js

@@ -76,6 +76,8 @@ export class ViewerPdfToolbarNewElement extends PolymerElement {
         type: Number,
         observer: 'viewportZoomChanged_',
       },
+      /** @type {!{min: number, max: number}} */
+      zoomBounds: Object,
 
       sidenavCollapsed: Boolean,
       twoUpViewEnabled: Boolean,
@@ -127,9 +129,6 @@ export class ViewerPdfToolbarNewElement extends PolymerElement {
 
     /** @private {boolean} */
     this.moreMenuOpen_ = false;
-
-    /** @private {?number} */
-    this.zoomTimeout_ = null;
   }
 
   /**
@@ -293,20 +292,25 @@ export class ViewerPdfToolbarNewElement extends PolymerElement {
   }
 
   /** @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 @@ export class ViewerPdfToolbarNewElement extends PolymerElement {
     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

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_]]"

chrome/browser/resources/pdf/pdf_viewer.js

@@ -181,6 +181,7 @@ export class PDFViewerElement extends PDFViewerBaseElement {
       pdfAnnotationsEnabled_: Boolean,
       printingEnabled_: Boolean,
       viewportZoom_: Number,
+      zoomBounds_: Object,
     };
   }
 
@@ -244,6 +245,9 @@ export class PDFViewerElement extends PDFViewerBaseElement {
     /** @private {number} */
     this.viewportZoom_ = 1;
 
+    /** @private {!{ min: number, max: number }} */
+    this.zoomBounds_ = {min: 0, max: 0};
+
     // Non-Polymer properties
 
     /** @type {number} */
@@ -763,6 +767,10 @@ export class PDFViewerElement extends PDFViewerBaseElement {
         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 */

chrome/browser/resources/pdf/viewport.js

@@ -501,6 +501,11 @@ export class Viewport {
     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();

chrome/test/data/pdf/viewer_pdf_toolbar_new_test.js

@@ -141,6 +141,7 @@ const tests = [
   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 @@ const tests = [
     toolbar.viewportZoom = .533;
     chrome.test.assertEq('53%', zoomField.value);
 
+    // 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.
-    let sentValue = -1;
-    toolbar.addEventListener('zoom-changed', e => {
-      sentValue = e.detail;
-      chrome.test.assertEq(110, sentValue);
+          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();
         });
-    zoomField.value = '110%';
-    zoomField.dispatchEvent(new CustomEvent('input'));
   },
 
   // Test that the overflow menu closes when an action is triggered.