Skip to content

Conversation

@arifulhoque7
Copy link
Contributor

@arifulhoque7 arifulhoque7 commented Oct 14, 2025

Close issue , Relate PRO PR

  • Enhanced wp_kses() to allow SVG elements and attributes
  • Added support for radio image field type rendering
  • Expanded allowed HTML tags to include: svg, path elements with required attributes
  • Allows div, a, span elements needed for image-based option selection

This update enables the login form layout customization feature by providing the necessary settings API infrastructure for visual layout selection with SVG icons in the admin interface.

Summary by CodeRabbit

  • New Features

    • Added extensive login-form customization options and a visual layout picker with image-based selection and pro-preview styling.
  • Bug Fixes

    • Improved sanitization for color inputs to preserve safe markup and prevent unsafe HTML.
    • Color picker initialization now correctly skips disabled fields.
  • UI Improvements

    • Redesigned remember-me area, added a prominent Lost Password link, and added disabled styling for pro-preview color controls.

- Enhanced wp_kses() to allow SVG elements and attributes
- Added support for radio image field type rendering
- Expanded allowed HTML tags to include: svg, path elements with required attributes
- Allows div, a, span elements needed for image-based option selection

This update enables the login form layout customization feature
by providing the necessary settings API infrastructure for
visual layout selection with SVG icons in the admin interface.
@arifulhoque7 arifulhoque7 self-assigned this Oct 14, 2025
@arifulhoque7 arifulhoque7 requested a review from sapayth October 14, 2025 06:34
@coderabbitai
Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

Sanitization for the color-picker HTML was tightened and initialization now skips disabled fields. Login form markup was restructured to group remember-me elements and add a Lost Password link. New admin CSS and login-form-settings styles were added. A set of login-form customization settings and two helper functions for layout options/rendering were introduced.

Changes

Cohort / File(s) Change summary
Settings API — color picker
Lib/WeDevs_Settings_API.php
Replaced wp_kses_post() usage with wp_kses() and an explicit allowed-tags/attributes whitelist for color-picker HTML; limited allowed input attributes (type, class, id, name, value, data-default-color, disabled). Also adjusted JS init to select .wp-color-picker-field:not([disabled]).
Color picker UI CSS
assets/css/admin.css
Added rules to visually disable and prevent interaction for color picker controls in pro-preview disabled state (reduced opacity, not-allowed cursor, disabled pointer-events, preserved visuals on hover/active).
Login form template
templates/login-form.php
Restructured remember-me area: wrapped checkbox+label in span.wpuf-remember-me, added span.wpuf-lost-password with a Lost Password link (wp_lostpassword_url()), changed outer paragraph class to forgetmenot wpuf-remember-forgot-wrapper, and included 'lostpassword' => false in action-links retrieval.
Login form settings CSS
assets/css/login-form-settings.css
Added comprehensive styles for a radio-image selection UI (hidden accessible radios, image thumbnails, selected-state badge/shadow), layout overrides for admin table fieldsets, and cross-browser layout enforcement using flex and important rules.
Settings fields additions
includes/functions/settings-options.php
Introduced many new login form customization settings (layout, background/border, field/label/placeholder/input/button colors, etc.), added a $crown_icon variable for pro badges, and integrated pro-preview callbacks for these fields.
Layout helper functions
wpuf-functions.php
Added wpuf_get_login_layout_options() to return layout keys/labels/image URLs and wpuf_render_login_layout_field( $args ) to render the radio-image layout selection field (with pro-preview handling and description).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Review focus:
    • Verify the wp_kses whitelist is sufficiently restrictive and includes only intended attributes.
    • Confirm JS selector change correctly ignores disabled color fields without breaking enabled ones.
    • Check accessibility of the restructured remember-me markup (label association, keyboard focus).
    • Validate new settings wiring and pro-preview indicators (correct option names, rendering output).
    • Inspect radio-image field rendering for correct IDs/names and image path construction.

Suggested reviewers

  • sapayth

Poem

I nibble tags and tidy quirks,
I paint the pickers, tweak the works.
A lost-link nestled by the gate,
Checkboxes cozy, layouts straight.
Hop-hop — a peek, a patch, delight! 🐇🎨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "feat: Update settings API for login form layout customization support" directly and accurately reflects the primary objective of the changeset. The PR summary indicates the core purpose is to introduce settings API infrastructure to enable login form layout customization with visual selection in the admin interface, which is exactly what the title conveys. The title is specific and descriptive—it clearly identifies what is being updated (settings API) and the purpose (login form layout customization support)—rather than being vague or generic. While the changeset includes multiple supporting changes across CSS, templates, and utility functions, they all serve this unifying objective, making the title a fair summary of the main change that a teammate scanning commit history would readily understand.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
Lib/WeDevs_Settings_API.php (1)

464-464: Verified pro preview HTML compatibility

  • wpuf_get_pro_preview_html() outputs only <div>, <a>, <span>, <svg>, and <path> elements with attributes fully covered by the explicit whitelist.
  • Optional: align callback_textarea (line 354) and callback_password (line 444) to use wp_kses with explicit whitelists for consistency.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35fec8b and 944903d.

📒 Files selected for processing (1)
  • Lib/WeDevs_Settings_API.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Lib/WeDevs_Settings_API.php (2)
includes/Render_Form.php (1)
  • html (1525-1531)
class/render-form.php (1)
  • html (1634-1640)
🪛 GitHub Actions: Inspections
Lib/WeDevs_Settings_API.php

[error] 1-1: Doc comment short description must start with a capital letter

🔇 Additional comments (1)
Lib/WeDevs_Settings_API.php (1)

1-11: Pipeline failure is a false positive.

The linter flags the doc comment for not starting with a capital letter, but "weDevs" is a proper name/brand name and should remain as-is. This is unrelated to the current PR changes and can be safely ignored or addressed separately.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 944903d and 06509a9.

📒 Files selected for processing (1)
  • templates/login-form.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
templates/login-form.php (1)
includes/Free/Simple_Login.php (1)
  • get_action_links (312-335)
🪛 GitHub Actions: Inspections
templates/login-form.php

[warning] 1-1: Found precision alignment of 2 spaces.


[error] 1-1: PHPCS: Expected 1 space after '!'; 0 found.

🪛 GitHub Check: Run PHPCS inspection
templates/login-form.php

[warning] 114-114:
When a multi-item array uses associative keys, each value should start on a new line.

🔇 Additional comments (2)
templates/login-form.php (2)

92-100: LGTM! Login form UX improved.

The restructuring enhances the user experience by:

  • Organizing remember-me and lost-password into a logical wrapper
  • Adding a lost-password link directly in the form for better accessibility
  • Using proper escaping (esc_url, esc_html_e) and translation

1-115: Include Lib/WeDevs_Settings_API.php changes. The PR diff includes both templates/login-form.php and Lib/WeDevs_Settings_API.php; please include the latter’s modifications in the review for completeness.

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

Fix formatting to comply with PHPCS standards.

Multi-item arrays with associative keys should have each value on a new line per WordPress coding standards.

Apply this diff:

-    <?php echo wp_kses_post( wpuf()->frontend->simple_login->get_action_links( [ 'login' => false, 'lostpassword' => false ] ) ); ?>
+    <?php
+    echo wp_kses_post(
+        wpuf()->frontend->simple_login->get_action_links(
+            [
+                'login'        => false,
+                'lostpassword' => false,
+            ]
+        )
+    );
+    ?>

As per coding guidelines.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[warning] 114-114:
When a multi-item array uses associative keys, each value should start on a new line.

🤖 Prompt for AI Agents
In templates/login-form.php around line 114, the inline short-array with
multiple associative items violates PHPCS WordPress standards; replace the
bracket-style array with a multi-line array() where each key => value pair is on
its own indented line, include a trailing comma after the last element, and
align the closing parenthesis so the call becomes
wpuf()->frontend->simple_login->get_action_links( array( 'login' => false,
'lostpassword' => false, ) ) but formatted across multiple lines with proper
indentation to satisfy the multi-item array rule.

@sapayth sapayth added Dev Review Done and removed needs: dev review This PR needs review by a developer labels Oct 20, 2025
@Rubaiyat-E-Mohammad Rubaiyat-E-Mohammad added QA Approved This PR is approved by the QA team and removed needs: testing labels Oct 22, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Lib/WeDevs_Settings_API.php (1)

444-444: Apply the same input tag sanitization fix to callback_password function.

The callback_password function generates an input field (line 437) but uses wp_kses_post() (line 444) which strips input tags. This is identical to the issue fixed in callback_color. Apply the same approach:

Location: Lib/WeDevs_Settings_API.php, lines 444

Replace:

        echo wp_kses_post( $html );

With:

        $allowed_html = wp_kses_allowed_html( 'post' );
        $allowed_html['input'] = [
            'type'     => true,
            'class'    => true,
            'id'       => true,
            'name'     => true,
            'value'    => true,
            'disabled' => true,
        ];

        echo wp_kses( $html, $allowed_html );
🧹 Nitpick comments (1)
Lib/WeDevs_Settings_API.php (1)

464-477: Good fix, but consider adding SVG support for consistency.

The change correctly fixes the input tag stripping issue by using wp_kses() with a custom allowed HTML array instead of wp_kses_post(). The implementation is secure and the allowed attributes properly match the generated HTML.

However, for consistency with other callback functions (callback_text, callback_number, callback_checkbox, etc.) and to ensure pro preview HTML renders correctly, consider adding SVG-related tags to the allowed HTML array.

Apply this diff to add SVG support:

     $allowed_html = wp_kses_allowed_html( 'post' );
     $allowed_html['input'] = [
         'type'               => true,
         'class'              => true,
         'id'                 => true,
         'name'               => true,
         'value'              => true,
         'data-default-color' => true,
         'disabled'           => true,
     ];
+    $allowed_html['svg'] = [
+        'width'   => true,
+        'height'  => true,
+        'viewBox' => true,
+        'fill'    => true,
+        'xmlns'   => true,
+    ];
+    $allowed_html['path'] = [
+        'd'    => true,
+        'fill' => true,
+    ];
 
     echo wp_kses( $html, $allowed_html );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06509a9 and 4e86f5f.

📒 Files selected for processing (1)
  • Lib/WeDevs_Settings_API.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Lib/WeDevs_Settings_API.php (2)
includes/Render_Form.php (1)
  • html (1525-1531)
class/render-form.php (1)
  • html (1634-1640)
🪛 GitHub Actions: Inspections
Lib/WeDevs_Settings_API.php

[warning] 1-1: Found precision alignment of 2 spaces.


[error] 1-1: PHPCS: Expected 1 space after '!'; 0 found.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
assets/css/login-form-settings.css (1)

22-37: Add focus-visible and reduced‑motion support for a11y

Keyboard users need a visible focus ring; and motion-sensitive users may prefer no hover animation.

Suggested additions:

+.wpuf-radio-image-option input[type='radio']:focus + label,
+.wpuf-radio-image-option input[type='radio']:focus-visible + label {
+  outline: 2px solid #0073aa;
+  outline-offset: 2px;
+  box-shadow: 0 0 0 2px rgba(0,115,170,0.25);
+}
+
+@media (prefers-reduced-motion: reduce) {
+  .wpuf-radio-image-option label,
+  .wpuf-radio-image-option label:hover,
+  .wpuf-radio-image-option input[type='radio']:checked + label {
+    transition: none;
+    transform: none;
+    box-shadow: none;
+  }
+}

Also applies to: 39-49, 51-68, 77-107

includes/functions/settings-options.php (1)

58-58: Optimize SVG icon retrieval: cache once instead of inline repeated reads

The crown icon is currently read 12 times via inline file_get_contents() calls (lines 471–551, 743) without error checking. Since WeDevs_Settings_API already sanitizes output with wp_kses (including SVG allowlist), add defensive error handling and cache the icon at initialization instead:

 function wpuf_settings_fields() {
-    $crown_icon = WPUF_ROOT . '/assets/images/crown.svg';
+    $crown_icon_path = WPUF_ROOT . '/assets/images/crown.svg';
+    $crown_icon      = file_exists( $crown_icon_path ) ? file_get_contents( $crown_icon_path ) : '';

Then reuse $crown_icon in all field labels. This avoids repeated I/O, adds resilience to missing files, and simplifies maintenance.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e86f5f and 17abb18.

⛔ Files ignored due to path filters (7)
  • assets/images/login-layouts/layout1.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout2.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout3.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout4.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout5.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout6.svg is excluded by !**/*.svg
  • assets/images/login-layouts/layout7.svg is excluded by !**/*.svg
📒 Files selected for processing (5)
  • Lib/WeDevs_Settings_API.php (2 hunks)
  • assets/css/admin.css (1 hunks)
  • assets/css/login-form-settings.css (1 hunks)
  • includes/functions/settings-options.php (2 hunks)
  • wpuf-functions.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Lib/WeDevs_Settings_API.php
🧰 Additional context used
🧬 Code graph analysis (1)
includes/functions/settings-options.php (1)
wpuf-functions.php (2)
  • wpuf_get_login_layout_options (5717-5739)
  • wpuf_is_pro_active (4990-4992)
🔇 Additional comments (3)
includes/functions/settings-options.php (1)

469-478: Pro‑preview gating looks good; double‑check default handling and disabled logic

  • Good: is_pro_preview => ! wpuf_is_pro_active() consistently used to gate layout + color fields.
  • Verify: the renderer should actually add disabled to inputs when is_pro_preview is true for all affected types, not only the custom callback. If this is centralized in WeDevs_Settings_API, we’re good.

No code changes needed here if the API behavior is confirmed.

Also applies to: 480-485, 487-492, 494-499, 501-506, 508-513, 515-520, 522-527, 529-534, 536-541, 543-548, 550-555

wpuf-functions.php (1)

5717-5739: LGTM on options helper

Clear labels and URLs; no issues spotted. Ensure the SVGs exist at WPUF_ASSET_URI/images/login-layouts/.

assets/css/admin.css (1)

1473-1494: LGTM: Disabled color picker states align with pro‑preview

Styles clearly communicate non-interactivity and match the expected [disabled] behavior.

Comment on lines +5748 to +5798
function wpuf_render_login_layout_field( $args ) {
if ( empty( $args['section'] ) || empty( $args['id'] ) || empty( $args['options'] ) ) {
return;
}

$value = get_option( $args['section'] );
$current = $value[ $args['id'] ] ?? $args['std'] ?? '';
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
$wrapper_class = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'pro-preview-html' : '';

echo '<fieldset>';

printf( '<div class="wpuf-radio-image-wrapper %s">', esc_attr( $wrapper_class ) );

foreach ( $args['options'] as $key => $option ) {
$checked = checked( $current, $key, false );
$label = esc_html( $option['label'] ?? $key );
$image = esc_url( $option['image'] ?? '' );

printf(
'<div class="wpuf-radio-image-option">
<input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
<label for="%1$s_%2$s" title="%6$s">',
esc_attr( $args['id'] ),
esc_attr( $key ),
esc_attr( $args['section'] ),
$checked,
$disabled,
$label
);

if ( $image ) {
printf( '<img src="%s" alt="%s">', $image, $label );
}

echo '</label></div>';
}

// Add pro preview overlay inside the wrapper
if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) {
echo wpuf_get_pro_preview_html();
}

echo '</div>';

if ( ! empty( $args['desc'] ) ) {
printf( '<p class="description">%s</p>', wp_kses_post( $args['desc'] ) );
}

echo '</fieldset>';
}
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

🧩 Analysis chain

Fix default fallback and attribute escaping in renderer

Two issues:

  • Default: you read $args['std'] but the field uses default. Initial selection may be empty.
  • Escaping: title and alt should use esc_attr(). Currently $label is esc_html only.

Apply:

 function wpuf_render_login_layout_field( $args ) {
@@
-    $value   = get_option( $args['section'] );
-    $current = $value[ $args['id'] ] ?? $args['std'] ?? '';
-    $disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
+    $value    = get_option( $args['section'] );
+    $current  = isset( $value[ $args['id'] ] )
+        ? $value[ $args['id'] ]
+        : ( $args['default'] ?? ( $args['std'] ?? '' ) );
+    $is_disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'];
@@
-    foreach ( $args['options'] as $key => $option ) {
+    foreach ( (array) $args['options'] as $key => $option ) {
         $checked = checked( $current, $key, false );
-        $label   = esc_html( $option['label'] ?? $key );
+        $label   = $option['label'] ?? $key;
+        $label_attr = esc_attr( $label );
+        $label_text = esc_html( $label );
         $image   = esc_url( $option['image'] ?? '' );
 
         printf(
             '<div class="wpuf-radio-image-option">
-                <input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
-                <label for="%1$s_%2$s" title="%6$s">',
+                <input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
+                <label for="%1$s_%2$s" title="%6$s">',
             esc_attr( $args['id'] ),
             esc_attr( $key ),
             esc_attr( $args['section'] ),
             $checked,
-            $disabled,
-            $label
+            $is_disabled ? 'disabled="disabled"' : '',
+            $label_attr
         );
 
         if ( $image ) {
-            printf( '<img src="%s" alt="%s">', $image, $label );
+            printf( '<img src="%s" alt="%s">', $image, $label_attr );
         }
 
-        echo '</label></div>';
+        echo wp_kses_post( '</label></div>' );
     }
@@
-    if ( ! empty( $args['desc'] ) ) {
-        printf( '<p class="description">%s</p>', wp_kses_post( $args['desc'] ) );
-    }
+    if ( ! empty( $args['desc'] ) ) {
+        printf( '<p class="description">%s</p>', wp_kses_post( $args['desc'] ) );
+    }

I'll help you verify the issues raised in this review comment. Let me search the codebase to understand the calling context and field definitions.


🏁 Script executed:

#!/bin/bash
# Find the wpuf-functions.php file and verify the function exists
find . -name "wpuf-functions.php" -type f | head -5

Length of output: 20


🏁 Script executed:

#!/bin/bash
# Search for calls to wpuf_render_login_layout_field to understand how $args is constructed
rg -n "wpuf_render_login_layout_field" --type=php -B 3 -A 3

Length of output: 1027


🏁 Script executed:

#!/bin/bash
# Search for where 'std' and 'default' keys are set in field definitions
rg -n "(\['std'\]|\['default'\])" --type=php -B 2 -A 2 | head -100

Length of output: 5511


🏁 Script executed:

#!/bin/bash
# Search for field option definitions to understand the parameter structure
rg -n "'id'\s*=>" --type=php -B 1 -A 5 | grep -E "(std|default)" | head -30

Length of output: 758


🏁 Script executed:

#!/bin/bash
# Look for field definitions specific to login layout rendering
rg -n "login.*layout.*field|login.*layout.*option" --type=php -i -B 2 -A 5 | head -50

Length of output: 2579


🏁 Script executed:

#!/bin/bash
# Check if options is ever passed as non-array to this function
rg -n "\['options'\]\s*=" --type=php -B 1 -A 1 | head -30

Length of output: 531


🏁 Script executed:

#!/bin/bash
# Verify current code escaping practices in the file - look for esc_attr usage
rg -n "esc_attr|esc_html" wpuf-functions.php --type=php -C 2 | head -60

Length of output: 3859


🏁 Script executed:

#!/bin/bash
# Search for where this rendering function is registered/hooked
rg -n "add_action.*wpuf_render|register.*wpuf_render" --type=php -B 1 -A 1

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for the specific function definition to confirm line numbers and current state
sed -n '5748,5798p' wpuf-functions.php

Length of output: 1696


🏁 Script executed:

#!/bin/bash
# Check if there are any admin field definitions using 'std' or 'default' keys
rg -n "'std'|'default'" --type=php -B 3 -A 1 | grep -E "(login|layout|radio)" -i | head -40

Length of output: 3269


🏁 Script executed:

#!/bin/bash
# Search for option defaults specifically in admin or field setup
rg -n "render_login_layout_field" --type=php -B 10 -A 10 | head -80

Length of output: 3278


🏁 Script executed:

#!/bin/bash
# Look for args array construction that might use 'std' or other keys
rg -n "\['options'\]|'section'|'is_pro_preview'" --type=php -B 3 -A 2 | head -80

Length of output: 5117


🏁 Script executed:

#!/bin/bash
# Search for where get_option might be used with field config
rg -n "get_option.*section" --type=php -B 2 -A 3 | head -40

Length of output: 2524


🏁 Script executed:

#!/bin/bash
# Look for sanitize/validate callbacks that might reveal field structure
rg -n "sanitize.*login|login.*sanitize" --type=php -i -B 2 -A 3 | head -40

Length of output: 2954


🏁 Script executed:

#!/bin/bash
# Check for actual field configuration arrays that call wpuf_render_login_layout_field
ast-grep --pattern $'[
  $$$
  "callback" => "wpuf_render_login_layout_field"
  $$$
]'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for register_setting or do_settings_fields context
rg -n "register_setting|do_settings_fields" --type=php -B 3 -A 5 | head -60

Length of output: 566


🏁 Script executed:

#!/bin/bash
# Look for where the radio image options are defined
rg -n "'image'|'label'|'options'" --type=php -B 2 -A 2 | grep -E "(login|layout)" -i | head -50

Length of output: 6138


🏁 Script executed:

#!/bin/bash
# Check for test files or examples that show how this function is called
find . -name "*test*.php" -o -name "*example*.php" | xargs grep -l "wpuf_render_login_layout_field" 2>/dev/null | head -5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for where field args are built with 'std' or 'default'
rg -n "std.*=>|default.*=>" --type=php -B 5 | grep -E "(login|field|render)" -i -A 3 | head -80

Length of output: 7307


🏁 Script executed:

#!/bin/bash
# Look at the field definitions more carefully around line 5748
rg -n "function wpuf_render_login_layout_field" --type=php -B 30 -A 60 | head -100

Length of output: 4538


🏁 Script executed:

#!/bin/bash
# Check for array casting patterns in the codebase
rg -n "\(array\)" --type=php | head -20

Length of output: 1512


🏁 Script executed:

#!/bin/bash
# Verify escaping best practices - search for esc_attr usage with title/alt
rg -n "title=|alt=" --type=php -B 1 -A 1 | grep "esc_" | head -20

Length of output: 3352


🏁 Script executed:

#!/bin/bash
# Check if disabled attribute is used elsewhere and how it's formatted
rg -n "disabled.*disabled|disabled=\"disabled\"" --type=php | head -15

Length of output: 1735


🏁 Script executed:

#!/bin/bash
# Look for wp_kses_post usage on closing tags
rg -n "wp_kses_post.*</" --type=php | head -10

Length of output: 1202


🏁 Script executed:

#!/bin/bash
# Check context around the function to understand the broader pattern
wc -l wpuf-functions.php
sed -n '5740,5810p' wpuf-functions.php

Length of output: 1830


🏁 Script executed:

#!/bin/bash
# Search for field definitions array that passes options to this callback
rg -n "callback.*render_login_layout_field" --type=php -B 50 | tail -80

Length of output: 5108


🏁 Script executed:

#!/bin/bash
# Look for the complete field definition context
rg -n "'id'\s*=>.*'section'\s*=>" --type=php -B 5 -A 15 | grep -A 20 "login.*layout" -i | head -60

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Final check - search for actual field hook/filter that provides these arguments
rg -n "apply_filters.*login.*layout|do_action.*login.*layout" --type=php -i -B 2 -A 5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search entire codebase for how this function args are passed
grep -rn "wpuf_render_login_layout_field" . --include="*.php" | head -20

Length of output: 192


🏁 Script executed:

#!/bin/bash
# Get the actual implementation by line range
head -5798 wpuf-functions.php | tail -51

Length of output: 1696


🏁 Script executed:

#!/bin/bash
# Look for field definitions in settings or admin pages
find . -name "*.php" -exec grep -l "login.*layout\|layout.*login" {} \; | head -10

Length of output: 62


🏁 Script executed:

#!/bin/bash
# Check if std vs default is documented or used consistently
grep -rn "'std'" . --include="*.php" | wc -l
grep -rn "'default'" . --include="*.php" | wc -l

Length of output: 6


🏁 Script executed:

#!/bin/bash
# Search for where field arguments are constructed for this specific callback
grep -rn "wpuf_render_login_layout_field" . --include="*.php" -B 30 | grep -E "(std|default|options|section)" | head -40

Length of output: 3342


🏁 Script executed:

#!/bin/bash
# Look at admin settings where this might be used
find . -path "*/admin*" -name "*.php" | xargs grep -l "login.*layout" -i 2>/dev/null | head -5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Final comprehensive search for the field definition
rg -n "render_login_layout_field|login-layout|login_layout" --type=php -B 10 -A 10 | head -150

Length of output: 4794


🏁 Script executed:

#!/bin/bash
# Check the actual arguments passed by finding the field registration
grep -rn "'callback'.*'wpuf_render_login_layout_field'" . --include="*.php" -B 20 -A 5 | head -100

Length of output: 2717


Address default parameter mismatch and attribute escaping vulnerabilities

The review comment correctly identifies two critical issues:

  1. Default parameter mismatch: The field definition (includes/functions/settings-options.php:475) provides 'default' => 'layout1', but the function (wpuf-functions.php:5754) reads $args['std'] which doesn't exist in this context. Initial selection will be empty.

  2. Attribute escaping: The title and alt attributes (lines 5770, 5780) use esc_html() for $label, but HTML attributes require esc_attr() per WordPress security standards.

  3. Disabled attribute format: The disabled attribute should be 'disabled="disabled"' (consistent with other fields at line 3722), not just 'disabled'.

Apply these fixes:

 function wpuf_render_login_layout_field( $args ) {
     if ( empty( $args['section'] ) || empty( $args['id'] ) || empty( $args['options'] ) ) {
         return;
     }
 
     $value   = get_option( $args['section'] );
-    $current = $value[ $args['id'] ] ?? $args['std'] ?? '';
-    $disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
+    $current = isset( $value[ $args['id'] ] )
+        ? $value[ $args['id'] ]
+        : ( $args['default'] ?? ( $args['std'] ?? '' ) );
+    $is_disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'];
 
     foreach ( $args['options'] as $key => $option ) {
         $checked = checked( $current, $key, false );
-        $label   = esc_html( $option['label'] ?? $key );
+        $label   = $option['label'] ?? $key;
+        $label_attr = esc_attr( $label );
+        $label_text = esc_html( $label );
         $image   = esc_url( $option['image'] ?? '' );
 
         printf(
             '<div class="wpuf-radio-image-option">
                 <input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
                 <label for="%1$s_%2$s" title="%6$s">',
             esc_attr( $args['id'] ),
             esc_attr( $key ),
             esc_attr( $args['section'] ),
             $checked,
-            $disabled,
-            $label
+            $is_disabled ? 'disabled="disabled"' : '',
+            $label_attr
         );
 
         if ( $image ) {
-            printf( '<img src="%s" alt="%s">', $image, $label );
+            printf( '<img src="%s" alt="%s">', $image, $label_attr );
         }
 
         echo '</label></div>';
     }
📝 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
function wpuf_render_login_layout_field( $args ) {
if ( empty( $args['section'] ) || empty( $args['id'] ) || empty( $args['options'] ) ) {
return;
}
$value = get_option( $args['section'] );
$current = $value[ $args['id'] ] ?? $args['std'] ?? '';
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : '';
$wrapper_class = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'pro-preview-html' : '';
echo '<fieldset>';
printf( '<div class="wpuf-radio-image-wrapper %s">', esc_attr( $wrapper_class ) );
foreach ( $args['options'] as $key => $option ) {
$checked = checked( $current, $key, false );
$label = esc_html( $option['label'] ?? $key );
$image = esc_url( $option['image'] ?? '' );
printf(
'<div class="wpuf-radio-image-option">
<input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
<label for="%1$s_%2$s" title="%6$s">',
esc_attr( $args['id'] ),
esc_attr( $key ),
esc_attr( $args['section'] ),
$checked,
$disabled,
$label
);
if ( $image ) {
printf( '<img src="%s" alt="%s">', $image, $label );
}
echo '</label></div>';
}
// Add pro preview overlay inside the wrapper
if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) {
echo wpuf_get_pro_preview_html();
}
echo '</div>';
if ( ! empty( $args['desc'] ) ) {
printf( '<p class="description">%s</p>', wp_kses_post( $args['desc'] ) );
}
echo '</fieldset>';
}
function wpuf_render_login_layout_field( $args ) {
if ( empty( $args['section'] ) || empty( $args['id'] ) || empty( $args['options'] ) ) {
return;
}
$value = get_option( $args['section'] );
$current = isset( $value[ $args['id'] ] )
? $value[ $args['id'] ]
: ( $args['default'] ?? ( $args['std'] ?? '' ) );
$is_disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'];
$wrapper_class = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'pro-preview-html' : '';
echo '<fieldset>';
printf( '<div class="wpuf-radio-image-wrapper %s">', esc_attr( $wrapper_class ) );
foreach ( $args['options'] as $key => $option ) {
$checked = checked( $current, $key, false );
$label = $option['label'] ?? $key;
$label_attr = esc_attr( $label );
$label_text = esc_html( $label );
$image = esc_url( $option['image'] ?? '' );
printf(
'<div class="wpuf-radio-image-option">
<input type="radio" id="%1$s_%2$s" name="%3$s[%1$s]" value="%2$s" %4$s %5$s>
<label for="%1$s_%2$s" title="%6$s">',
esc_attr( $args['id'] ),
esc_attr( $key ),
esc_attr( $args['section'] ),
$checked,
$is_disabled ? 'disabled="disabled"' : '',
$label_attr
);
if ( $image ) {
printf( '<img src="%s" alt="%s">', $image, $label_attr );
}
echo '</label></div>';
}
// Add pro preview overlay inside the wrapper
if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) {
echo wpuf_get_pro_preview_html();
}
echo '</div>';
if ( ! empty( $args['desc'] ) ) {
printf( '<p class="description">%s</p>', wp_kses_post( $args['desc'] ) );
}
echo '</fieldset>';
}
🤖 Prompt for AI Agents
In wpuf-functions.php around lines 5748-5798, fix three issues: replace the
non-existent $args['std'] fallback when determining $current with
$args['default'] (i.e. $current = $value[$args['id']] ?? $args['default'] ??
''), change the disabled marker to the full attribute disabled="disabled" when
is_pro_preview is true (and ensure it's passed through esc_attr when printed),
and use esc_attr() (not esc_html()) for $label when outputting it into HTML
attributes (title and img alt). Ensure all inserted attribute values are
properly escaped with esc_attr() and image URL remains esc_url().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dev Review Done QA Approved This PR is approved by the QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants