usercss config: skip and warn about mismatching vars on saving

_locales/en/messages.json

@@ -973,6 +973,9 @@
   "usercssReplaceTemplateConfirmation": {
     "message": "Replace the default template for new Usercss styles with the current code?"
   },
+  "usercssConfigIncomplete": {
+    "message": "The style was updated or deleted after the configuration dialog was shown. These variables were not saved to avoid corrupting the style's metadata:"
+  },
   "versionInvalidOlder": {
     "message": "The version is older than the installed style.",
     "description": "Displayed when the version of style is older than the installed one"

manage/config-dialog.js

@@ -2,9 +2,15 @@
 'use strict';
 
 function configDialog(style) {
-  const form = buildConfigForm();
+  const varsHash = deepCopy(style.usercssData.vars) || {};
+  const varNames = Object.keys(varsHash);
+  const vars = varNames.map(name => varsHash[name]);
+  const elements = [];
   const colorpicker = window.colorpicker();
 
+  buildConfigForm();
+  renderValues();
+
   return messageBox({
     title: `${style.name} v${style.usercssData.version}`,
     className: 'config-dialog',
@@ -19,31 +25,67 @@ function configDialog(style) {
       }),
       $element({
         className: 'config-body',
-        appendChild: form.elements
+        appendChild: elements
       })
     ],
     buttons: [
       t('confirmSave'),
       {
         textContent: t('confirmDefault'),
-        onclick: form.useDefault
+        onclick: useDefault
       },
       t('confirmCancel')
     ]
-  }).then(({button, enter}) => {
+  }).then(({button, esc}) => {
     if (button !== 1) {
       colorpicker.hide();
     }
-    if (button === 0 || enter) {
+    if (button > 0 || esc || !vars.length || !vars.some(va => va.dirty)) {
-    return form.getVars();
+      return;
     }
+    style.reason = 'config';
+    const styleVars = style.usercssData.vars;
+    const bgStyle = BG.cachedStyles.byId.get(style.id);
+    const bgVars = bgStyle && (bgStyle.usercssData || {}).vars || {};
+    const invalid = [];
+    let numValid = 0;
+    for (const va of vars) {
+      const bgva = bgVars[va.name];
+      let error;
+      if (!bgva) {
+        error = `${va.name}: deleted`;
+      } else
+      if (bgva.type !== va.type) {
+        error = `${va.name}: type '${va.type}' != '${bgva.type}'`;
+      } else
+      if ((va.type === 'select' || va.type === 'dropdown') &&
+          bgva.options.every(o => o.name !== va.value)) {
+        error = `${va.name}: '${va.value}' not in the updated '${va.type}' list`;
+      } else if (!va.dirty) {
+        continue;
+      } else {
+        styleVars[va.name].value = va.value;
+        numValid++;
+        continue;
+      }
+      invalid.push(error);
+      delete styleVars[va.name];
+    }
+    if (invalid.length) {
+      messageBox.alert([
+        $element({textContent: t('usercssConfigIncomplete'), style: 'max-width: 34em'}),
+        $element({
+          tag: 'ol',
+          style: 'text-align: left; font-weight: bold;',
+          appendChild: invalid.map(s => $element({tag: 'li', textContent: s})),
+        }),
+      ]);
+    }
+    return numValid && BG.usercssHelper.save(style);
   });
 
   function buildConfigForm() {
-    const labels = [];
+    for (const va of vars) {
-    const vars = deepCopy(style.usercssData.vars);
-    for (const key of Object.keys(vars)) {
-      const va = vars[key];
       let appendChild;
       switch (va.type) {
         case 'color':
@@ -52,7 +94,7 @@ function configDialog(style) {
             appendChild: va.inputColor = $element({
               va,
               className: 'color-swatch',
-              onclick: onColorClicked,
+              onclick: showColorpicker,
             })
           })];
           break;
@@ -97,22 +139,26 @@ function configDialog(style) {
           appendChild = [va.input];
           break;
       }
-      appendChild.unshift($element({tag: 'span', appendChild: va.label}));
+      elements.push($element({
-      labels.push($element({
         tag: 'label',
         className: `config-${va.type}`,
-        appendChild
+        appendChild: [
+          $element({tag: 'span', appendChild: va.label}),
+          ...appendChild,
+        ],
       }));
     }
-    drawValues();
+  }
 
-    function drawValues() {
+  function renderValues() {
-      for (const key of Object.keys(vars)) {
+    for (const va of vars) {
-        const va = vars[key];
       const useDefault = va.value === null || va.value === undefined;
       const value = useDefault ? va.default : va.value;
       if (va.type === 'color') {
         va.inputColor.style.backgroundColor = value;
+        if (colorpicker.options.va === va) {
+          colorpicker.setColor(value);
+        }
       } else if (va.type === 'checkbox') {
         va.input.checked = Number(value);
       } else {
@@ -122,46 +168,40 @@ function configDialog(style) {
   }
 
   function useDefault() {
-      for (const key of Object.keys(vars)) {
+    for (const va of vars) {
-        const va = vars[key];
+      const hasValue = va.value !== null && va.value !== undefined;
-        va.dirty = va.value !== null && va.value !== undefined && va.value !== va.default;
+      va.dirty = hasValue && va.value !== va.default;
       va.value = null;
     }
-      drawValues();
+    renderValues();
   }
 
-    function getVars() {
+  function showColorpicker() {
-      return vars;
-    }
-
-    function onColorClicked() {
     window.removeEventListener('keydown', messageBox.listeners.key, true);
     const box = $('#message-box-contents');
     colorpicker.show({
+      va: this.va,
       color: this.va.value || this.va.default,
       top: this.getBoundingClientRect().bottom - 5,
       left: box.getBoundingClientRect().left - 360,
       hideDelay: 1e6,
       guessBrightness: box,
-        callback: newColor => {
+      callback: onColorChanged,
-          if (newColor) {
-            this.va.dirty = true;
-            this.va.value = newColor;
-            this.style.backgroundColor = newColor;
-          }
-          setTimeout(() => {
-            if (!$('.colorpicker-popup')) {
-              window.addEventListener('keydown', messageBox.listeners.key, true);
-            }
-          });
-        },
     });
   }
 
-    return {
+  function onColorChanged(newColor) {
-      elements: labels,
+    if (newColor) {
-      useDefault,
+      this.va.dirty = true;
-      getVars
+      this.va.value = newColor;
-    };
+      this.va.inputColor.style.backgroundColor = newColor;
+    }
+    debounce(restoreEscInDialog);
+  }
+
+  function restoreEscInDialog() {
+    if (!$('.colorpicker-popup') && messageBox.element) {
+      window.addEventListener('keydown', messageBox.listeners.key, true);
+    }
   }
 }

manage/manage.js

@@ -194,7 +194,7 @@ function createStyleElement({style, name}) {
   if (style.updateUrl && newUI.enabled) {
     $('.actions', entry).appendChild(template.updaterIcons.cloneNode(true));
   }
-  if (shouldShowConfig() && newUI.enabled) {
+  if (style.usercssData && Object.keys(style.usercssData.vars).length > 0 && newUI.enabled) {
     $('.actions', entry).appendChild(template.configureIcon.cloneNode(true));
   }
 
@@ -203,10 +203,6 @@ function createStyleElement({style, name}) {
   createStyleTargetsElement({entry, style, postponeFavicons: name});
 
   return entry;
-
-  function shouldShowConfig() {
-    return style.usercssData && Object.keys(style.usercssData.vars).length > 0;
-  }
 }
 
 
@@ -287,24 +283,6 @@ Object.assign(handleEvent, {
     '.configure-usercss': 'config'
   },
 
-  config(event, {styleMeta: style}) {
-    configDialog(style).then(vars => {
-      if (!vars) {
-        return;
-      }
-      const keys = Object.keys(vars).filter(k => vars[k].dirty);
-      if (!keys.length) {
-        return;
-      }
-      style.reason = 'config';
-      for (const key of keys) {
-        style.usercssData.vars[key].value = vars[key].value;
-      }
-      onBackgroundReady()
-        .then(() => BG.usercssHelper.save(style));
-    });
-  },
-
   entryClicked(event) {
     const target = event.target;
     const entry = target.closest('.entry');
@@ -407,6 +385,10 @@ Object.assign(handleEvent, {
       }
     }
   },
+
+  config(event, {styleMeta}) {
+    configDialog(styleMeta);
+  },
 });