glFusion / glfusion

glFusion CMS - Advanced Content Management with Style
https://www.glfusion.org
GNU General Public License v2.0
14 stars 15 forks source link

Configuration - Keyed array field doesn't work if there are no elements #481

Closed leegarner closed 2 years ago

leegarner commented 3 years ago

To reproduce, delete all elements from a keyed configuration array (type *select). When adding the first element, a JS error is thrown and the configuration.php page refreshes.

configmanager.js:44 Uncaught TypeError: Cannot read properties of undefined (reading 'cloneNode')
    at add_select (configmanager.js:44)
    at handleAddWithName (configmanager.js:33)
    at handleAdd (configmanager.js:15)
    at HTMLButtonElement.onclick (configuration.php:841)

The offending line is: dropDown = tbl.getElementsByTagName('tr')[0].getElementsByTagName('td')[1].getElementsByTagName('select')[0].cloneNode(true);

It appears that it's trying to clone a select node that doesn't yet exist. Observed in glFusion 2.0.0 and 1.7.9pl2

leegarner commented 3 years ago

Really hacky way to get the configuration un-broken, but there has to be a better way

--- a/private/system/classes/config.class.php
+++ b/private/system/classes/config.class.php
@@ -1174,6 +1174,10 @@ class config
                                            'unkeyed-add-button'));
             $t->set_var('my_add_element_button', $button);
             $result = "";
+            if ($type == '*select' && (!is_array($val) || empty($val))) {
+                $val = array('default' => 0);
+            }
+
             if ( is_array($val) ) {
                 if ($type == '%select') {
leegarner commented 2 years ago

Another way I've approached something similar where the selection depends on another selection is to populate a JSON var with the options, or even one var with the whole <select></select value, and put that into the innerHTML for additional options.

mark0263 commented 2 years ago

Honestly, that old JS needs to be completely thrown away and rewritten. I'll play around with it a little and see if it jogs my memory on the ins / outs of it all. If you have some better options - feel free :)

leegarner commented 2 years ago

I'm thinking the same thing. The ideas I've come up with are a) populate the configuration.thtml template with a JSON array of selection options like


var select_opts = {
     "varname1": '<select name="varname1"><option value="abc">abc</option><option value="def">def</option></select>',
     "varname2": '<select name="varname2"><option value="123">123</option><option value="456">456</option></select>'
}
Then set the innerHTML of the newly-created selection field from the array.

b) use AJAX to get the selection array from glFusion in realtime. Could be easier, not sure yet, but slower.

c) maybe a hybrid, keep clonine row 0 if it exists, otherwise use AJAX to get the initial contents. Might actually be simplest to implement.
leegarner commented 2 years ago

A mockup changing the cloneNode call in add_select in configmanager.js works ok. I'm thinking mayabe an AJAX call in the "else" clause to get the selection based on arr_name

    var varName = arr_name + "[" + index + "]";
    d = tbl.getElementsByTagName('tr')[0].getElementsByTagName('td')[1].getElementsByTagName('select')[0];
    if (typeof(d) != 'undefined') {
        dropDown = d.cloneNode(true);
        dropDown.name = varName;
        paramCell.appendChild(dropDown);
    } else {
        // todo: get Selection via AJAX
        var Selection = '<select name="' + varName + '"><option value="123">First</option>' +
            '<option value="345">Second</option></select';
        paramCell.innerHTML = Selection;
    }
leegarner commented 2 years ago

A prototype that works. Have two issues still, 1: getting the plugin name passed into add_select(), 2: handling options from the language file, probably need to set and pass $selectionArray as an alternative.


--- a/public_html/javascript/configmanager.js
+++ b/public_html/javascript/configmanager.js
@@ -40,25 +40,64 @@ function add_select(tbl, arr_name, index, deletable) {
     paramCell = newRow.insertCell(1);
     titleCell.className = "uk-text-left";
     titleCell.appendChild(document.createTextNode(index));
-    dropDown = tbl.getElementsByTagName('tr')[0].getElementsByTagName('td')[1].getElementsByTagName('select')[0].cloneNode(true);
-    dropDown.name = arr_name + "[" + index + "]";
-    paramCell.appendChild(dropDown);
+    //dropDown = tbl.getElementsByTagName('tr')[0].getElementsByTagName('td')[1].getElementsByTagName('select')[0].cloneNode(true);
+    d = tbl.getElementsByTagName('tr')[0].getElementsByTagName('td')[1].getElementsByTagName('select')[0];
+       var varName = arr_name + "[" + index + "]";
+       if (typeof(d) != 'undefined') {
+           dropDown = d.cloneNode(true);
+               dropDown.name = varName;
+               paramCell.appendChild(dropDown);
+           if (deletable) {
+                       addDeleteBtn(paramCell);
+           }
+       } else {
+           var dataS = {
+                   "action" : "configselectoptions",
+                       "plugin": 'evlist',         //<<<< Note the test group name
+                       "varname": arr_name
+               };
+               data = $.param(dataS);
+           $.ajax({
+                   type: "POST",
+                       dataType: "text",
+               url: site_admin_url + "/ajax_controller.php",
+                   data: data,
+                       success: function(result) {
+                               console.log("Ajax Result: " + result);
+                               paramCell.innerHTML = '<select name="' + varName + '">' +
+                                       result + '</select>';
+                           if (deletable) {
+                                       addDeleteBtn(paramCell);
+                           }
+                   },
+                       failure: function(x, y, z) {
+                               console.log(x);
+                               console.log(y);
+                               console.log(z);
+                       }
+           });
+       }

-    if (deletable) {
-        paramCell.appendChild(document.createTextNode("\n"));
-        deleteButton = document.createElement("button");
-        deleteButton.type = "button";
-        deleteButton.classList.add('uk-button');
-        deleteButton.classList.add('uk-button-danger');
-        deleteButton.classList.add('uk-button-small');
-        deleteButton.innerHTML = 'x';
-        deleteButton.value = "x";
-        deleteButton.onclick =
-            function () {
-                glfremove(this)
-            };
-        paramCell.appendChild(deleteButton);
-    }
+}
+
+/**
+ * Add a delete button to a field
+ */
+function addDeleteBtn(paramCell)
+{
+    paramCell.appendChild(document.createTextNode("\n"));
+       deleteButton = document.createElement("button");
+       deleteButton.type = "button";
+       deleteButton.classList.add('uk-button');
+       deleteButton.classList.add('uk-button-danger');
+       deleteButton.classList.add('uk-button-small');
+       deleteButton.innerHTML = 'x';
+       deleteButton.value = "x";
+       deleteButton.onclick =
+       function () {
+               glfremove(this)
+       };
+       paramCell.appendChild(deleteButton);
 }

--- a/public_html/admin/ajax_controller.php
+++ b/public_html/admin/ajax_controller.php
@@ -56,7 +56,23 @@ if (is_ajax()) {
             case 'pagepreview' :
                 pagePreview();
                 break;
-
+            case 'configselectoptions' :
+                $plugin = $_POST['plugin'];
+                $varname = $_POST['varname'];
+                $function = 'plugin_configmanager_select_' . $varname . '_' . $plugin;
+                $opt_str = '';
+                if (function_exists($function)) {
+                    $opts = $function();
+                    // Now format as a string of options
+                    if (is_array($opts)) {
+                        foreach ($opts as $dscp=>$val) {
+                            $opt_str .= '<option value="' . $val . '">' . $dscp . '</option>' . LB;
+                        }
+                    }
+                }
+                echo $opt_str;
+                exit;
+                break;
         }