Fix modifier key condition data (#506)

* save transformed data for modifier key conditions

* validate transformed input

* fix regression

* undo rename

* refactor transformInput handling

* don't overwrite value with null

3 files changed, 55 insertions, 20 deletions

diff --git a/ext/bg/js/conditions.js b/ext/bg/js/conditions.js
index eb9582df..3f3c0a45 100644
--- a/ext/bg/js/conditions.js
+++ b/ext/bg/js/conditions.js
@@ -32,7 +32,15 @@ function conditionsValidateOptionValue(object, value) {
     return value;
 }
 
-function conditionsNormalizeOptionValue(descriptors, type, operator, optionValue) {
+function conditionsValidateOptionInputValue(object, value) {
+    if (hasOwn(object, 'transformInput')) {
+        return object.transformInput(value);
+    }
+
+    return null;
+}
+
+function conditionsNormalizeOptionValue(descriptors, type, operator, optionValue, isInput) {
     if (!hasOwn(descriptors, type)) {
         throw new Error('Invalid type');
     }
@@ -44,13 +52,34 @@ function conditionsNormalizeOptionValue(descriptors, type, operator, optionValue
 
     const operatorDescriptor = conditionDescriptor.operators[operator];
 
-    let transformedValue = conditionsValidateOptionValue(conditionDescriptor, optionValue);
-    transformedValue = conditionsValidateOptionValue(operatorDescriptor, transformedValue);
+    const descriptorArray = [conditionDescriptor, operatorDescriptor];
+
+    let transformedValue = optionValue;
+
+    let inputTransformedValue = null;
+    if (isInput) {
+        for (const descriptor of descriptorArray) {
+            let value = inputTransformedValue !== null ? inputTransformedValue : transformedValue;
+            value = conditionsValidateOptionInputValue(descriptor, value);
+            if (value !== null) {
+                inputTransformedValue = value;
+            }
+        }
+
+        if (inputTransformedValue !== null) {
+            transformedValue = inputTransformedValue;
+        }
+    }
+
+    for (const descriptor of descriptorArray) {
+        transformedValue = conditionsValidateOptionValue(descriptor, transformedValue);
+    }
 
     if (hasOwn(operatorDescriptor, 'transformReverse')) {
         transformedValue = operatorDescriptor.transformReverse(transformedValue);
     }
-    return transformedValue;
+
+    return [transformedValue, inputTransformedValue];
 }
 
 function conditionsTestValueThrowing(descriptors, type, operator, optionValue, value) {
diff --git a/ext/bg/js/profile-conditions.js b/ext/bg/js/profile-conditions.js
index c0f5d3f5..32309c64 100644
--- a/ext/bg/js/profile-conditions.js
+++ b/ext/bg/js/profile-conditions.js
@@ -128,9 +128,9 @@ const profileConditionsDescriptor = {
             are: {
                 name: 'are',
                 placeholder: 'Press one or more modifier keys here',
-                defaultValue: '',
+                defaultValue: [],
                 type: 'keyMulti',
-                transform: (optionValue) => optionValue
+                transformInput: (optionValue) => optionValue
                     .split(' + ')
                     .filter((v) => v.length > 0)
                     .map((v) => _profileModifierNameToValue.get(v)),
@@ -142,9 +142,9 @@ const profileConditionsDescriptor = {
             areNot: {
                 name: 'are not',
                 placeholder: 'Press one or more modifier keys here',
-                defaultValue: '',
+                defaultValue: [],
                 type: 'keyMulti',
-                transform: (optionValue) => optionValue
+                transformInput: (optionValue) => optionValue
                     .split(' + ')
                     .filter((v) => v.length > 0)
                     .map((v) => _profileModifierNameToValue.get(v)),
diff --git a/ext/bg/js/settings/conditions-ui.js b/ext/bg/js/settings/conditions-ui.js
index 5b356101..0670de5a 100644
--- a/ext/bg/js/settings/conditions-ui.js
+++ b/ext/bg/js/settings/conditions-ui.js
@@ -104,11 +104,11 @@ ConditionsUI.Container = class Container {
             if (hasOwn(conditionDescriptor.operators, operator)) {
                 const operatorDescriptor = conditionDescriptor.operators[operator];
                 if (hasOwn(operatorDescriptor, 'defaultValue')) {
-                    return {value: operatorDescriptor.defaultValue, fromOperator: true};
+                    return {value: this.isolate(operatorDescriptor.defaultValue), fromOperator: true};
                 }
             }
             if (hasOwn(conditionDescriptor, 'defaultValue')) {
-                return {value: conditionDescriptor.defaultValue, fromOperator: false};
+                return {value: this.isolate(conditionDescriptor.defaultValue), fromOperator: false};
             }
         }
         return {fromOperator: false};
@@ -205,6 +205,10 @@ ConditionsUI.Condition = class Condition {
         this.parent.save();
     }
 
+    isolate(object) {
+        return this.parent.isolate(object);
+    }
+
     updateTypes() {
         const conditionDescriptors = this.parent.parent.conditionDescriptors;
         const optionGroup = this.typeSelect.find('optgroup');
@@ -266,9 +270,9 @@ ConditionsUI.Condition = class Condition {
         this.inputInner.appendTo(this.input);
         this.inputInner.on('change', this.onInputChanged.bind(this));
 
-        const {valid} = this.validateValue(this.condition.value);
+        const {valid, value} = this.validateValue(this.condition.value);
         this.inputInner.toggleClass('is-invalid', !valid);
-        this.inputInner.val(this.condition.value);
+        this.inputInner.val(value);
     }
 
     createInputElement(objects) {
@@ -366,7 +370,7 @@ ConditionsUI.Condition = class Condition {
                 data.set('values', object.values);
             }
             if (hasOwn(object, 'defaultValue')) {
-                data.set('defaultValue', object.defaultValue);
+                data.set('defaultValue', this.isolate(object.defaultValue));
             }
         }
 
@@ -379,33 +383,35 @@ ConditionsUI.Condition = class Condition {
 
         const defaultValue = data.get('defaultValue');
         if (defaultValue !== null) {
-            inputInner.val(defaultValue);
+            inputInner.val(this.isolate(defaultValue));
         }
 
         return inputInner;
     }
 
-    validateValue(value) {
+    validateValue(value, isInput=false) {
         const conditionDescriptors = this.parent.parent.conditionDescriptors;
         let valid = true;
+        let inputTransformedValue = null;
         try {
-            value = conditionsNormalizeOptionValue(
+            [value, inputTransformedValue] = conditionsNormalizeOptionValue(
                 conditionDescriptors,
                 this.condition.type,
                 this.condition.operator,
-                value
+                value,
+                isInput
             );
         } catch (e) {
             valid = false;
         }
-        return {valid, value};
+        return {valid, value, inputTransformedValue};
     }
 
     onInputChanged() {
-        const {valid, value} = this.validateValue(this.inputInner.val());
+        const {valid, value, inputTransformedValue} = this.validateValue(this.inputInner.val(), true);
         this.inputInner.toggleClass('is-invalid', !valid);
         this.inputInner.val(value);
-        this.condition.value = value;
+        this.condition.value = inputTransformedValue !== null ? inputTransformedValue : value;
         this.save();
     }