Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 165 additions & 22 deletions Lib/WeDevs_Settings_API.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
function admin_enqueue_scripts() {
wp_enqueue_script( 'jquery' );

if ( isset( $_GET['page'] ) && ( $_GET['page'] == 'wpuf-settings' || $_GET['page'] == 'wpuf-post-forms' || $_GET['page'] == 'wpuf-modules' || $_GET['page'] == 'wpuf-profile-forms' ) ) {

Check warning on line 39 in Lib/WeDevs_Settings_API.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Processing form data without nonce verification.

Check warning on line 39 in Lib/WeDevs_Settings_API.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="

Check warning on line 39 in Lib/WeDevs_Settings_API.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Processing form data without nonce verification.

Check warning on line 39 in Lib/WeDevs_Settings_API.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Processing form data without nonce verification.
wp_enqueue_media();
wp_enqueue_style( 'wp-color-picker' );
wp_enqueue_script( 'wp-color-picker' );
Expand Down Expand Up @@ -141,6 +141,7 @@
'step' => isset( $option['step'] ) ? $option['step'] : '',
'is_pro_preview' => ! empty( $option['is_pro_preview'] ) ? $option['is_pro_preview'] : false,
'depends_on' => ! empty( $option['depends_on'] ) ? $option['depends_on'] : '',
'depends_on_value' => ! empty( $option['depends_on_value'] ) ? $option['depends_on_value'] : '',
);

add_settings_field( $section . '[' . $option['name'] . ']', $option['label'], (isset($option['callback']) ? $option['callback'] : array($this, 'callback_' . $type )), $section, $section, $args );
Expand Down Expand Up @@ -180,18 +181,27 @@
$placeholder = empty( $args['placeholder'] ) ? '' : ' placeholder="' . $args['placeholder'] . '"';
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
$depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : '';
$depends_on_value = ! empty( $args['depends_on_value'] ) ? $args['depends_on_value'] : '';

// Handle array dependencies
if (is_array($depends_on)) {
$depends_on_json = esc_attr( json_encode($depends_on) );
$depends_on_value = ''; // Not used for array format
} else {
$depends_on_json = esc_attr( $depends_on );
}

$html = sprintf(
'<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" />',
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on
'<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on=\'%8$s\' data-depends-on-value="%9$s" />',
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on_json, esc_attr( $depends_on_value )
);
$html .= $this->get_field_description( $args );

if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) {
$html .= wpuf_get_pro_preview_html();
}

echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing 'placeholder' attribute in wp_kses allowed attributes.

The placeholder attribute is used in the sprintf on line 195 (set on line 181), but it's not included in the wp_kses allowed attributes list on line 204. This will cause the placeholder to be stripped from the output.

Apply this diff to add the missing attribute:

-        echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
+        echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'placeholder' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'placeholder' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
🤖 Prompt for AI Agents
In Lib/WeDevs_Settings_API.php around line 204, the wp_kses allowed-attributes
list for the input tag is missing the 'placeholder' attribute causing
placeholder text (set earlier) to be stripped; update the allowed attributes
array for 'input' to include 'placeholder' => [] so wp_kses will preserve
placeholder values in the rendered HTML.

}

/**
Expand Down Expand Up @@ -372,8 +382,18 @@
$value = esc_attr( $this->get_option( $args['id'], $args['section'], $args['std'] ) );
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
$size = isset( $args['size'] ) && ! is_null( $args['size'] ) ? $args['size'] : 'regular';
$depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : '';
$depends_on_value = ! empty( $args['depends_on_value'] ) ? $args['depends_on_value'] : '';

// Handle array dependencies
if (is_array($depends_on)) {
$depends_on_json = esc_attr( json_encode($depends_on) );
$depends_on_value = ''; // Not used for array format
} else {
$depends_on_json = esc_attr( $depends_on );
}

$html = sprintf( '<select class="%1$s" name="%2$s[%3$s]" id="%2$s[%3$s]" %4$s>', $size, $args['section'], $args['id'], $disabled );
$html = sprintf( '<select class="%1$s" name="%2$s[%3$s]" id="%2$s[%3$s]" %4$s data-depends-on=\'%5$s\' data-depends-on-value="%6$s">', $size, $args['section'], $args['id'], $disabled, $depends_on_json, esc_attr( $depends_on_value ) );

foreach ( $args['options'] as $key => $label ) {
$html .= sprintf( '<option value="%s"%s>%s</option>', $key, selected( $value, $key, false ), $label );
Expand All @@ -386,7 +406,7 @@
$html .= wpuf_get_pro_preview_html();
}

echo wp_kses( $html, array('select' => ['class' => [], 'name' => [], 'id' => [], 'disabled' => []], 'option' => ['value' => [], 'selected' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [], 'target' => [], 'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [], 'height' => [], 'viewBox' => [], 'fill' => [], 'xmlns' => []], 'path' => ['d' => [], 'fill' => [] ] ) );
echo wp_kses( $html, array('select' => ['class' => [], 'name' => [], 'id' => [], 'disabled' => [], 'data-depends-on' => [], 'data-depends-on-value' => []], 'option' => ['value' => [], 'selected' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [], 'target' => [], 'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [], 'height' => [], 'viewBox' => [], 'fill' => [], 'xmlns' => []], 'path' => ['d' => [], 'fill' => [] ] ) );
}

/**
Expand Down Expand Up @@ -796,35 +816,158 @@
fields.each(function() {
var $this = $(this);
var data_depends_on = $this.data('depends-on');
var data_depends_on_value = $this.data('depends-on-value');

if (!data_depends_on) {
return;
}

var $depends_on = $("input[id*='"+ data_depends_on +"']");
var $depends_on_type = $depends_on.attr('type');
// Handle multiple dependencies
var dependencies = {};
if (typeof data_depends_on === 'string' && data_depends_on.startsWith('{')) {
// JSON string format: '{"field1": "value1", "field2": "value2"}'
try {
dependencies = JSON.parse(data_depends_on);
} catch (e) {
return;
}
} else if (typeof data_depends_on === 'object') {
// Multiple dependencies: {field1: 'value1', field2: 'value2'}
dependencies = data_depends_on;
} else {
// Single dependency: field_name => expected_value
dependencies[data_depends_on] = data_depends_on_value;
}

if ($depends_on_type === 'checkbox') {
if ($depends_on.is(':checked')) {
$this.closest('tr').show();
} else {
$this.closest('tr').hide();
// Check all dependencies
var all_dependencies_met = true;
var dependency_fields = {};

for (var field_name in dependencies) {
var expected_value = dependencies[field_name];
var $depends_on = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']");

// If no field found with the simple selector, try more specific selectors
if ($depends_on.length === 0) {
$depends_on = $("input[name*='["+ field_name +"]'], select[name*='["+ field_name +"]']");
}

// If still no field found, try looking for the field name in the ID attribute
if ($depends_on.length === 0) {
$depends_on = $("input[id*='["+ field_name +"]'], select[id*='["+ field_name +"]']");
}

// If still no field found, try looking for the field name anywhere in the ID
if ($depends_on.length === 0) {
$depends_on = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']");
}

if ($depends_on.length === 0) {
all_dependencies_met = false;
break;
}
$depends_on.on('change', function() {
if ($(this).is(':checked')) {
$this.closest('tr').show();

dependency_fields[field_name] = $depends_on;
var current_value = $depends_on.val();
var is_checkbox = $depends_on.attr('type') === 'checkbox';
var is_checked = is_checkbox ? $depends_on.is(':checked') : null;

// For checkboxes, handle empty string as "checked" expectation
var value_matches = false;
if (is_checkbox) {
if (expected_value === '' || expected_value === null || expected_value === undefined) {
// Empty expected value means "show when checked" (typical use case)
value_matches = is_checked;
} else {
$this.closest('tr').hide();
// Non-empty expected value means "show when checked with this value"
value_matches = is_checked && (current_value === expected_value || expected_value === 'on');
}
});
} else {
value_matches = current_value === expected_value;
}

if (!value_matches) {
all_dependencies_met = false;
}
}
Comment on lines +870 to +892
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Radio button value comparison broken.

For radio buttons, .val() on a jQuery collection returns the value attribute of the first element, not the checked one. This prevents radio-based dependencies from working correctly.

The checkbox handling partially addresses the previous review's concerns, but radio buttons still need special treatment. Apply this diff to fix radio button handling in both the initial check and checkAllDependencies():

In the initial dependency check (around line 870):

                         dependency_fields[field_name] = $depends_on;
                         var current_value = $depends_on.val();
                         var is_checkbox = $depends_on.attr('type') === 'checkbox';
+                        var is_radio = $depends_on.attr('type') === 'radio';
                         var is_checked = is_checkbox ? $depends_on.is(':checked') : null;
                         
                         // For checkboxes, handle empty string as "checked" expectation
                         var value_matches = false;
                         if (is_checkbox) {
                             if (expected_value === '' || expected_value === null || expected_value === undefined) {
                                 // Empty expected value means "show when checked" (typical use case)
                                 value_matches = is_checked;
                             } else {
                                 // Non-empty expected value means "show when checked with this value"
                                 value_matches = is_checked && (current_value === expected_value || expected_value === 'on');
                             }
+                        } else if (is_radio) {
+                            // For radio buttons, get the value of the checked one
+                            var radio_name = $depends_on.attr('name');
+                            var checked_value = $('input[name="'+ radio_name +'"]:checked').val() || '';
+                            value_matches = checked_value === expected_value;
                         } else {
                             value_matches = current_value === expected_value;
                         }

In checkAllDependencies (around line 940):

                             var current_value = $depends_on.length > 0 ? $depends_on.val() : 'NOT_FOUND';
                             var is_checkbox = $depends_on.length > 0 && $depends_on.attr('type') === 'checkbox';
+                            var is_radio = $depends_on.length > 0 && $depends_on.attr('type') === 'radio';
                             var is_checked = is_checkbox ? $depends_on.is(':checked') : null;
                             
                             // For checkboxes, handle empty string as "checked" expectation
                             var value_matches = false;
                             if ($depends_on.length === 0) {
                                 value_matches = false;
                             } else if (is_checkbox) {
                                 if (expected_value === '' || expected_value === null || expected_value === undefined) {
                                     // Empty expected value means "show when checked" (typical use case)
                                     value_matches = is_checked;
                                 } else {
                                     // Non-empty expected value means "show when checked with this value"
                                     value_matches = is_checked && (current_value === expected_value || expected_value === 'on');
                                 }
+                            } else if (is_radio) {
+                                // For radio buttons, get the value of the checked one
+                                var radio_name = $depends_on.attr('name');
+                                var checked_value = $('input[name="'+ radio_name +'"]:checked').val() || '';
+                                value_matches = checked_value === expected_value;
                             } else {
                                 value_matches = current_value === expected_value;
                             }

Also applies to: 940-964

🤖 Prompt for AI Agents
In Lib/WeDevs_Settings_API.php around lines 870-892 and 940-964, radio-button
dependency checks use $depends_on.val() which returns the first element's value
instead of the checked radio's value; update both places to detect radio groups
(e.g. $depends_on.is(':radio') or $depends_on.filter(':radio').length) and when
radio inputs are present retrieve the value of the checked radio via
$depends_on.filter(':checked').val() (and treat absence as unchecked), then
perform comparisons against expected_value (mirroring checkbox logic: empty
expected_value means show when checked, otherwise compare checked value or
'on'), ensuring you branch for radio vs checkbox vs other inputs in both the
initial check and in checkAllDependencies().


// Show/hide based on all dependencies
if (all_dependencies_met) {
$this.closest('tr').show();
} else {
$depends_on.on('keyup change', function() {
if ($(this).val() === $this.data('depends-on-value')) {
$this.closest('tr').show();
$this.closest('tr').hide();
}

// Set up event handlers for all dependency fields
for (var field_name in dependency_fields) {
var $depends_on = dependency_fields[field_name];
var expected_value = dependencies[field_name];
var $depends_on_type = $depends_on.attr('type');

if ($depends_on_type === 'checkbox') {
$depends_on.on('change', function() {
checkAllDependencies();
});
} else {
$depends_on.on('keyup change', function() {
checkAllDependencies();
});
}
}

// Function to check all dependencies
function checkAllDependencies() {
var all_met = true;
for (var field_name in dependencies) {
var expected_value = dependencies[field_name];
var $depends_on = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']");

// If no field found with the simple selector, try more specific selectors
if ($depends_on.length === 0) {
$depends_on = $("input[name*='["+ field_name +"]'], select[name*='["+ field_name +"]']");
}

// If still no field found, try looking for the field name in the ID attribute
if ($depends_on.length === 0) {
$depends_on = $("input[id*='["+ field_name +"]'], select[id*='["+ field_name +"]']");
}

// If still no field found, try looking for the field name anywhere in the ID
if ($depends_on.length === 0) {
$depends_on = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']");
}

var current_value = $depends_on.length > 0 ? $depends_on.val() : 'NOT_FOUND';
var is_checkbox = $depends_on.length > 0 && $depends_on.attr('type') === 'checkbox';
var is_checked = is_checkbox ? $depends_on.is(':checked') : null;

// For checkboxes, handle empty string as "checked" expectation
var value_matches = false;
if ($depends_on.length === 0) {
value_matches = false;
} else if (is_checkbox) {
if (expected_value === '' || expected_value === null || expected_value === undefined) {
// Empty expected value means "show when checked" (typical use case)
value_matches = is_checked;
} else {
// Non-empty expected value means "show when checked with this value"
value_matches = is_checked && (current_value === expected_value || expected_value === 'on');
}
} else {
$this.closest('tr').hide();
value_matches = current_value === expected_value;
}
});

if (!value_matches) {
all_met = false;
break;
}
}

if (all_met) {
$this.closest('tr').show();
} else {
$this.closest('tr').hide();
}
}
});
});
Expand Down
Loading
Loading