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

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

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('print_preview', function() {
   'use strict';
 
   /**
    * Component that renders the scaling settings UI.
    * @param {!print_preview.ticket_items.scalingTicketItem} scalingTicketItem
@@ skipping 105 matching lines @@
     isFitToPageSelected: function() {
       return this.fitToPageTicketItem_.isCapabilityAvailable() &&
              this.fitToPageTicketItem_.getValue();
     },
 
     /**
      * @return {number} The current input field value as an integer.
      * @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();
     },
 
     /**
      * Display the fit to page scaling in the scaling field if there is a valid
      * fit to page scaling value. If not, make the field blank.
      * @private
      */
     displayFitToPageScaling: function() {
       this.inputField_.value = this.fitToPageScaling_ || '';
     },
 
     /**
+     * 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.
      */
     updateFitToPageScaling: function(fitToPageScaling) {
       this.fitToPageScaling_ = fitToPageScaling;
       // Display the new value if fit to page is checked.
       if (this.isFitToPageSelected())
         this.displayFitToPageScaling();
     },
 
     /**
      * Updates the state of the scaling settings UI controls.
      * @private
      */
     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 display matches, mark valid
+        // and return.
+        if (this.isFitToPageSelected() && this.displayMatchesFitToPage()) {
+          this.inputField_.classList.remove('invalid');
+          fadeOutElement(this.getChildElement('.hint'));
+          this.updateUiStateInternal();
+          return;
         }
 
-        // 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 == '')))) {
-          this.inputField_.classList.remove('invalid');
-          fadeOutElement(this.getChildElement('.hint'));
-        } else {
-          // Invalid value. Display the hint.
+        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();
     },
 
     /**
      * Helper function to adjust the scaling display if fit to page is checked
      * by the user.
      * @private
      */
     onFitToPageChange_: function() {
       if (this.isFitToPageSelected()) {
         // 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();
       }
     },
 
     /**
      * Called after a timeout after user input into the textfield.
      * @private
      */
     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));
     },
 
     /**
      * Called when a key is pressed on the custom input.
      * @param {!Event} event Contains the key that was pressed.
      * @private
      */
     onTextfieldKeyDown_: function(event) {
       if (event.keyCode != 'Enter')
         return;
@@ skipping 17 matching lines @@
           ScalingSettings.TEXTFIELD_DELAY_MS_);
     },
 
     /**
      * Called when the focus leaves the textfield. If the textfield is empty,
      * its value is set to the fit to page value if fit to page is checked and
      * 100 otherwise.
      * @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 {
           if (this.scalingTicketItem_.getValue() == '100')
             // No need to update the ticket, but change the display to match.
             this.inputField_.value = '100';
           else
             this.scalingTicketItem_.updateValue('100');
         }
       }
     },
 
   };
 
   // Export
   return {
     ScalingSettings: ScalingSettings
   };
 });