Print Preview: Show error message if text is typed in Copies section

chrome/browser/resources/print_preview/settings/scaling_settings.js

Index: chrome/browser/resources/print_preview/settings/scaling_settings.js
diff --git a/chrome/browser/resources/print_preview/settings/scaling_settings.js b/chrome/browser/resources/print_preview/settings/scaling_settings.js
index 9ac5438c86f70d6c0348727119c7060bf6ecf5a8..64a63ff918b0867ca7e8fbb1811f089cb5f547e2 100644
--- a/chrome/browser/resources/print_preview/settings/scaling_settings.js
+++ b/chrome/browser/resources/print_preview/settings/scaling_settings.js
@@ -123,9 +123,7 @@ cr.define('print_preview', function() {
      * @private
      */
     getInputAsNumber: function() {
-      return (/[^\d]+/.test(this.inputField_.value)) ?
-          0 : // Return an invalid scaling so that the hint will display.
-          parseInt(this.inputField_.value, 10);
+      return Number(this.inputField_.value).valueOf();
     },
 
     /**
@@ -138,6 +136,15 @@ cr.define('print_preview', function() {
     },
 
     /**
+     * Whether the displayed scaling value matches the fit to page scaling.
+     * @private
+     */
+    displayMatchesFitToPage: function() {
+      return (this.getInputAsNumber() == this.fitToPageScaling_ ||
+              (this.inputField_.value == '' && !this.fitToPageScaling_));
+    },
+
+    /**
      * Updates the fit to page scaling value of the scalings settings UI.
      * @param {number} fitToPageScaling The updated percentage scaling required
      *     to fit the document to page.
@@ -156,34 +163,25 @@ cr.define('print_preview', function() {
     updateState_: function() {
       if (this.isAvailable()) {
         var displayedValue = this.getInputAsNumber();
-        var inputString = this.inputField_.value;
-        // Display should be updated to match the ticket item unless displayed
-        // value is invalid and non-blank or fit to page is checked. In these
-        // cases, the ticket item is not updated and retains its last valid,
-        // user specified value.
-        if ((inputString == '' ||
-             (displayedValue !== this.scalingTicketItem_.getValueAsNumber() &&
-              this.scalingTicketItem_.wouldValueBeValid(inputString))) &&
-            !this.isFitToPageSelected()) {
-          this.inputField_.value = this.scalingTicketItem_.getValue();
-          displayedValue = this.getInputAsNumber();
-          inputString = this.inputField_.value;
-        }
-
-        // If fit to page is selected and the value matches the fit to page
-        // scaling, or if the value is valid, no hint is displayed
-        if (this.scalingTicketItem_.wouldValueBeValid(inputString) ||
-            (this.isFitToPageSelected() &&
-             (displayedValue == this.fitToPageScaling_ ||
-              (!this.fitToPageScaling_ && inputString == '')))) {
+        // If fit to page is selected and the display matches, mark valid
+        // and return.
+        if (this.isFitToPageSelected() && this.displayMatchesFitToPage()) {
           this.inputField_.classList.remove('invalid');
           fadeOutElement(this.getChildElement('.hint'));
-        } else {
-          // Invalid value. Display the hint.
+          this.updateUiStateInternal();
+          return;
+        }
+
+        if (!this.inputField_.validity.valid) {
           this.inputField_.classList.add('invalid');
           fadeInElement(this.getChildElement('.hint'));
+          this.updateUiStateInternal();
+          return;
         }
 
+        this.inputField_.value = this.scalingTicketItem_.getValue();
+        this.inputField_.classList.remove('invalid');
+        fadeOutElement(this.getChildElement('.hint'));
       }
       this.updateUiStateInternal();
     },
@@ -198,8 +196,7 @@ cr.define('print_preview', function() {
         // Fit to page was checked. Set scaling to the fit to page scaling.
         this.displayFitToPageScaling();
       } else if (this.fitToPageTicketItem_.isCapabilityAvailable() &&
-        (this.getInputAsNumber() == this.fitToPageScaling_ ||
-         this.inputField_.value == '')) {
+                 this.displayMatchesFitToPage()) {
         // Fit to page unchecked. Return to last scaling.
         this.inputField_.value = this.scalingTicketItem_.getValue();
       }
@@ -211,25 +208,25 @@ cr.define('print_preview', function() {
      */
     onTextfieldTimeout_: function() {
       this.textfieldTimeout_ = null;
-      var scalingValue = this.inputField_.value;
-      if (scalingValue == '')
+      if (!this.inputField_.validity.valid){
+        this.updateState_();
         return;
-      if (this.scalingTicketItem_.wouldValueBeValid(scalingValue)) {
-        if (scalingValue === this.scalingTicketItem_.getValue()) {
-          // Make sure this still gets called in case returning to a valid
-          // value.
-          this.updateState_();
-        }
-        if (this.isFitToPageSelected() &&
-            parseInt(scalingValue, 10) != this.fitToPageScaling_) {
-          // User modified value away from fit to page.
-          this.fitToPageTicketItem_.updateValue(false);
-        }
-        this.scalingTicketItem_.updateValue(scalingValue);
-        this.inputField_.value = scalingValue;
-      } else {
+      }
+      if (this.isFitToPageSelected() && !this.displayMatchesFitToPage()) {
+        // User modified value away from fit to page.
+        this.fitToPageTicketItem_.updateValue(false);
+      }
+      // Check this after checking for fit to page in case the user went
+      // back to their old value.
+      if (Number(this.inputField_.value).toFixed(0) ===

[dpapad, 2017/02/28 00:36:42]

Is the change from parseInt() to Number(...).toFixed() or Number(...).valueOf()
necessary in this CL?

The docs don't state the Number() constructor is supposed to parse non numerical
values, so (even though it seems to work) I would be hesitant to use it that
way. Can we revert those changes if they are not necessary?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

In addition to that there seems to exist a convenient valueAsNumber field on the
native <input>, so perhaps we could use that directly and avoid using Number()
or parseInt() completely.

[rbpotter, 2017/02/28 02:34:05]

Done. Switched this to use the valueAsNumber field. The purpose of
this change was to deal with scientific notation. The number input
type accepts scientific notation (e.g. 1e1 instead of 10) but
parseInt() fails on these values and parses only until it gets to
the "e" (so 1e1 returns 1). valueAsNumber seems to work however.

+          this.scalingTicketItem_.getValue()) {
         this.updateState_();
+        return;
       }
+      if (this.inputField_.value == '')
+        return;
+      this.scalingTicketItem_.updateValue(
+        Number(this.inputField_.value).toFixed(0));
     },
 
     /**
@@ -267,7 +264,7 @@ cr.define('print_preview', function() {
      * @private
      */
     onTextfieldBlur_: function() {
-      if (this.inputField_.value == '') {
+      if (this.inputField_.value == '' && this.inputField_.validity.valid) {

[dpapad, 2017/02/27 21:56:02]

I am slightly confused about this condition. It seems that the two clauses can
never be true at the same time, can they? Same question for Copiessettings.

[rbpotter, 2017/02/28 00:12:59]

They can actually both be true. This is the case for a truly empty
input, because I did not set required=true in the HTML. This allows
the behavior to stay the same as before (where a blank field does
not result in an error message). If the input is invalid then the
value will be returned as '' but validity.valid will be false. So
the combined condition selects only the case where the user has
an entirely blank input.

[dpapad, 2017/02/28 00:36:42]

Thanks for the explanation.

         if (this.isFitToPageSelected()) {
           this.displayFitToPageScaling();
         } else {