storage: Don't change dialog state during input validation

Author: mvollmer

pkg/storaged/dialog.jsx

@@ -420,11 +420,11 @@ export const dialog_open = (def) => {
     function run_action(progress_callback, variant) {
         const func = () => {
             return validate(variant)
-                    .then(() => {
+                    .then(validated_values => {
                         const visible_values = { variant };
                         fields.forEach(f => {
                             if (is_visible(f, values))
-                                visible_values[f.tag] = values[f.tag];
+                                visible_values[f.tag] = validated_values[f.tag];
                         });
                         if (def.Action.wrapper)
                             return def.Action.wrapper(visible_values, progress_callback,
@@ -506,16 +506,32 @@ export const dialog_open = (def) => {
     };
 
     const validate = (variant) => {
+        // The validation functions sometimes change the dialog values
+        // for the benefit of the action functions. For example, a
+        // SizeSlider will convert from "text plus unit" to a numeric
+        // value during validation.
+        //
+        // However, if the action fails, we don't want the values in
+        // the dialog to change.  The SizeSlider would convert back
+        // from a numeric value to "text plus unit", for example, and
+        // that conversion might change what the user had type.
+        //
+        // So we make a copy of the dialog state and let the validate
+        // and action functions work with that.
+        //
+        const validated_values = { ...values };
+
         return Promise.all(fields.map(f => {
             if (is_visible(f, values) && f.options && f.options.validate)
-                return f.options.validate(values[f.tag], values, variant);
+                return f.options.validate(validated_values[f.tag], validated_values, variant);
             else
                 return null;
         })).then(results => {
             const errors = { };
             fields.forEach((f, i) => { if (results[i]) errors[f.tag] = results[i]; });
             if (Object.keys(errors).length > 0)
                 return Promise.reject(errors);
+            return validated_values;
         });
     };
 

test/verify/check-storage-partitions

@@ -93,23 +93,21 @@ class TestStoragePartitions(storagelib.StorageCase):
 
         # Switch the dialog into the mode where it stores separate text and unit
         # instead of a numeric value.
-        self.dialog_set_val("size", 26.2)
-
-        # Change to GB
-        unit = "1000000000"
-        b.select_from_dropdown(".size-unit > select", unit)
+        b.set_input_text(".size-text > input", "26.200001")
+        b.select_from_dropdown(".size-unit > select", "1000000000")  # GB
 
         # Apply the dialog. This will convert from text+unit back to a
         # correctly rounded numeric value (which is too large).
 
         self.dialog_apply()
         self.dialog_wait_error("size", "Size is too large")
 
+        # The text in the input field must not have changed
+        b.wait_val(".size-text > input", "26.200001")
+
         # Change unit back to MB.  (This must round again, which it
         # didn't always do.)
-
-        unit = "1000000"
-        b.select_from_dropdown(".size-unit > select", unit)
+        b.select_from_dropdown(".size-unit > select", "1000000")  # MB
 
         self.dialog_apply()
         self.dialog_wait_close()