Merge pull request #180 from craftcms/bugfix/152-js-config-warn-about…

…-functions warn about callback fn when switching from js to json config
craftcms · Mar 6, 2024 · 16ccbfa · 16ccbfa
2 parents 0fcc9b0 + 5c2fda9
commit 16ccbfa
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Release Notes for CKEditor for Craft CMS
 
+## Unreleased
+
+- CKEditor config edit pages now warn when switching the Config Options setting from JavaScript to JSON if the JavaScript code contains any functions. ([#152](https://github.com/craftcms/ckeditor/issues/152), [#180](https://github.com/craftcms/ckeditor/pull/180))
+
 ## 3.8.0 - 2024-02-21
 
 - Added support for creating anchor links. ([#169](https://github.com/craftcms/ckeditor/discussions/169))

diff --git a/src/translations/en/ckeditor.php b/src/translations/en/ckeditor.php
@@ -42,6 +42,7 @@
     'View available settings' => 'View available settings',
     'Word Limit' => 'Word Limit',
     'You can save custom {name} configs as {ext} files in {path}.' => 'You can save custom {name} configs as {ext} files in {path}.',
+    'Your JavaScript config contains functions. If you switch to JSON, they will be lost. Would you like to continue?',
     '{attribute} isn’t valid JSON.' => '{attribute} isn’t valid JSON.',
     '{field} should contain at most {max, number} {max, plural, one{word} other{words}}.' => '{field} should contain at most {max, number} {max, plural, one{word} other{words}}.',
 ];
diff --git a/src/web/assets/ckeconfig/dist/ckeconfig.js b/src/web/assets/ckeconfig/dist/ckeconfig.js
diff --git a/src/web/assets/ckeconfig/dist/ckeconfig.js.map b/src/web/assets/ckeconfig/dist/ckeconfig.js.map
diff --git a/src/web/assets/ckeconfig/src/ConfigOptions.js b/src/web/assets/ckeconfig/src/ConfigOptions.js
@@ -35,21 +35,53 @@ export default Garnish.Base.extend({
 
     this.defaults = {};
 
+    let lastJsValue = null;
+
     new Craft.Listbox($languagePicker, {
       onChange: ($selectedOption) => {
         this.language = $selectedOption.data('language');
         switch (this.language) {
           case 'json':
+            // get the js value
+            lastJsValue = this.jsEditor.getModel().getValue();
+            // check if the js value has any functions in it
+            if (this.jsContainsFunctions(lastJsValue)) {
+              // if it does - show the confirmation dialogue
+              if (
+                !confirm(
+                  Craft.t(
+                    'ckeditor',
+                    'Your JavaScript config contains functions. If you switch to JSON, they will be lost. Would you like to continue?',
+                  ),
+                )
+              ) {
+                // if user cancels - go back to the previous option (js)
+                let listbox = $languagePicker.data('listbox');
+                listbox.$options.not('[data-language="json"]').trigger('click');
+                break;
+              }
+            }
+            // if user confirms that they want to proceed, or we don't have functions in the js value,
+            // go ahead and switch
             this.$jsonContainer.removeClass('hidden');
             this.$jsContainer.addClass('hidden');
-            const json = this.js2json(this.jsEditor.getModel().getValue());
+            const json = this.js2json(lastJsValue);
+            lastJsValue = null;
             this.jsonEditor.getModel().setValue(json || '{\n  \n}');
             this.jsEditor.getModel().setValue('');
             break;
           case 'js':
             this.$jsonContainer.addClass('hidden');
             this.$jsContainer.removeClass('hidden');
-            const js = this.json2js(this.jsonEditor.getModel().getValue());
+            let js;
+            // if we have the last remembered js value, it means we're switching back after cancelled confirmation,
+            // so let's use it
+            if (lastJsValue !== null) {
+              js = lastJsValue;
+              lastJsValue = null;
+            } else {
+              js = this.json2js(this.jsonEditor.getModel().getValue());
+            }
             this.jsEditor.getModel().setValue(js || 'return {\n  \n}');
             this.jsonEditor.getModel().setValue('');
             break;
@@ -204,6 +236,27 @@ export default Garnish.Base.extend({
     this.defaults[setting] = schema.$defs[defName].default;
   },
 
+  replacer: function (key, value) {
+    if (typeof value === 'function') {
+      return '__HAS__FUNCTION__';
+    }
+    return value;
+  },
+
+  jsContainsFunctions: function (js) {
+    let config = this.getValidJsonConfig(js);
+    if (config === false) {
+      return true;
+    }
+
+    let json = JSON.stringify(config, this.replacer, 2);
+    if (json.match(/__HAS__FUNCTION__/)) {
+      return true;
+    }
+
+    return false;
+  },
+
   config2json: function (config) {
     let json = JSON.stringify(config, null, 2);
     if (json === '{}') {
@@ -212,7 +265,7 @@ export default Garnish.Base.extend({
     return json;
   },
 
-  js2json: function (js) {
+  getValidJsonConfig: function (js) {
     const m = (js || '').match(/return\s*(\{[\w\W]*})/);
     if (!m) {
       return false;
@@ -225,6 +278,17 @@ export default Garnish.Base.extend({
       // oh well
       return false;
     }
+
+    return config;
+  },
+
+  js2json: function (js) {
+    let config = this.getValidJsonConfig(js);
+
+    if (config === false) {
+      return false;
+    }
+
     return this.config2json(config);
   },