Skip to content

style custom css option #2600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

style custom css option #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc54c69
style custom css option
Liviu-p Nov 26, 2025
a611e06
merge master
Liviu-p Nov 26, 2025
aa4e30f
Merge branch 'master' into feature/ff-style-custom-css
Crabcyborg Nov 27, 2025
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
30 changes: 23 additions & 7 deletions classes/controllers/FrmStylesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,12 +803,20 @@ private static function manage_styles() {
* Echo content for the Custom CSS page.
*
* @param string $message
* @param array $extra_args An array of extra arguments.
* @return void
*/
public static function custom_css( $message = '' ) {
$settings = self::enqueue_codemirror();
$id = $settings ? 'frm_codemirror_box' : 'frm_custom_css_box';
$custom_css = self::get_custom_css();
public static function custom_css( $message = '', $extra_args = array() ) {
$id = $extra_args['id'] ?? 'frm_codemirror_box';
$settings = self::enqueue_codemirror( $id, $extra_args['placeholder'] ?? '' );
$id = $settings ? $id : 'frm_custom_css_box';
$show_errors = $extra_args['show_errors'] ?? true;
$custom_css = $extra_args['custom_css'] ?? self::get_custom_css();
$heading = $extra_args['heading'] ?? __( 'You can add custom css here or in your theme style.css. Any CSS added here will be used anywhere the Formidable CSS is loaded.', 'formidable' );
$textarea_params = ! empty( $extra_args['textarea_params'] ) ? $extra_args['textarea_params'] : array(
'name' => 'frm_custom_css',
'id' => $id,
);

include FrmAppHelper::plugin_path() . '/classes/views/styles/custom_css.php';
}
Comment on lines +806 to 822
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the enqueue_codemirror method and how $id is used
rg -n "enqueue_codemirror" -A 35 classes/controllers/FrmStylesController.php | head -60

Repository: Strategy11/formidable-forms

Length of output: 2598

🏁 Script executed:

#!/bin/bash
# Get the complete enqueue_codemirror method including the JavaScript part
sed -n '878,891p' classes/controllers/FrmStylesController.php

Repository: Strategy11/formidable-forms

Length of output: 337

🏁 Script executed:

#!/bin/bash
# Search for all calls to custom_css method
rg -n "custom_css\(" -B 3 -A 3 | head -100

Repository: Strategy11/formidable-forms

Length of output: 3407

🏁 Script executed:

#!/bin/bash
# View the full call to custom_css in _quick-settings.php
sed -n '258,275p' classes/views/styles/_quick-settings.php

Repository: Strategy11/formidable-forms

Length of output: 580

🏁 Script executed:

#!/bin/bash
# Search more thoroughly for all custom_css calls to see if id is user-controlled
rg -n "FrmStylesController::custom_css|->custom_css" -B 5 -A 15

Repository: Strategy11/formidable-forms

Length of output: 5022

🏁 Script executed:

#!/bin/bash
# Search for any AJAX handlers or admin actions that might call custom_css indirectly
rg -n "add_action|add_filter" classes/controllers/FrmStylesController.php

Repository: Strategy11/formidable-forms

Length of output: 276

🏁 Script executed:

#!/bin/bash
# Search for where _quick-settings.php is included
rg -n "_quick-settings.php" -B 3 -A 3

Repository: Strategy11/formidable-forms

Length of output: 3711

🏁 Script executed:

#!/bin/bash
# Search for where _style-options.php is included
rg -n "_style-options.php" -B 3 -A 3

Repository: Strategy11/formidable-forms

Length of output: 887

🏁 Script executed:

#!/bin/bash
# Check if there are any dynamic constructions of extra_args or calls to custom_css
rg -n "extra_args|\\$id.*=" -B 2 -A 2 classes/controllers/FrmStylesController.php | head -50

Repository: Strategy11/formidable-forms

Length of output: 1681

🏁 Script executed:

#!/bin/bash
# Let's analyze the JavaScript generation more carefully
# The sprintf generates JavaScript code with $id embedded in three places
# Let's check if there's any escaping or validation

# Look at the exact enqueue_codemirror implementation again
sed -n '858,891p' classes/controllers/FrmStylesController.php

echo "---"
echo "Now let's check if there's any sanitization of \$id parameter"
echo "---"

# Search for any sanitization functions applied to $id
rg -n "sanitize|esc_|wp_kses" classes/controllers/FrmStylesController.php | grep -i "id\|codemirror" || echo "No sanitization found on \$id"

Repository: Strategy11/formidable-forms

Length of output: 2594

🏁 Script executed:

#!/bin/bash
# Final check: search for any dynamic construction of extra_args array
rg -n "extra_args\s*=" -B 5 -A 10

Repository: Strategy11/formidable-forms

Length of output: 3170

Based on my verification, I found that while the security concern about $id is technically valid, the actual risk is lower than the original review suggested. Here are my findings:

Vulnerability Status:

  • The $id parameter is indeed used unsanitized in JavaScript generation at lines 882-884
  • However, the only call site in the codebase (_quick-settings.php:258) passes a hardcoded safe value: 'frm_single_style_custom_css'
  • The method is public but only callable from admin pages with frm_change_settings capability

Recommendation:
The code should add sanitization to $id for defense-in-depth, since the method is public and could be called by extensions with user-controlled data.

Sanitize the $id parameter before using it in JavaScript context.

The $id parameter extracted from $extra_args at line 810 is embedded directly into JavaScript at lines 882-884 without sanitization. While the current call site uses a hardcoded safe value, the public method signature allows external callers to pass arbitrary values. For defense-in-depth, use wp_kses_js_entities() or esc_attr() to sanitize $id before passing it to sprintf().

Static analysis warnings about unused variables ($message, $show_errors, $custom_css, $heading, $textarea_params) are false positives—these are used in the view included at line 821.

🧰 Tools
🪛 PHPMD (2.15.0)

809-809: Avoid unused parameters such as '$message'. (undefined)

(UnusedFormalParameter)

813-813: Avoid unused local variables such as '$show_errors'. (undefined)

(UnusedLocalVariable)

814-814: Avoid unused local variables such as '$custom_css'. (undefined)

(UnusedLocalVariable)

815-815: Avoid unused local variables such as '$heading'. (undefined)

(UnusedLocalVariable)

816-816: Avoid unused local variables such as '$textarea_params'. (undefined)

(UnusedLocalVariable)

Expand All @@ -820,7 +828,12 @@ public static function custom_css( $message = '' ) {
*
* @return string
*/
public static function get_custom_css() {
public static function get_custom_css( $single_style_settings = null ) {
// If the single style settings are passed, return the custom CSS from the single style settings.
if ( ! empty( $single_style_settings['single_style_custom_css'] ) && ! empty( $single_style_settings['enable_style_custom_css'] ) ) {
return $single_style_settings['single_style_custom_css'];
}

$settings = FrmAppHelper::get_settings();
if ( is_string( $settings->custom_css ) ) {
return $settings->custom_css;
Expand All @@ -842,7 +855,7 @@ public static function get_custom_css() {
*
* @return array|false
*/
private static function enqueue_codemirror() {
private static function enqueue_codemirror( $id = 'frm_codemirror_box', $placeholder = '' ) {
Copy link
Contributor

@Crabcyborg Crabcyborg Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've removed an exception now for the PHPCS rule that checks for missing @param comments.

It should be throwing errors for this now.

I'll wait to review this PR again until we have all of the workflows passing. I merged master, which has some fixes for PHPUnit / e2e test issues. Everything failing now should be specific to this PR.

if ( ! function_exists( 'wp_enqueue_code_editor' ) ) {
// The WordPress version is likely older than 4.9.
return false;
Expand All @@ -857,6 +870,7 @@ private static function enqueue_codemirror() {
// As the codemirror box only appears once you click into the Custom CSS tab, we need to auto-refresh.
// Otherwise the line numbers all end up with a 1px width causing overlap issues with the text in the content.
'autoRefresh' => true,
'placeholder' => $placeholder,
),
)
);
Expand All @@ -865,7 +879,9 @@ private static function enqueue_codemirror() {
wp_add_inline_script(
'code-editor',
sprintf(
'jQuery( function() { wp.codeEditor.initialize( \'frm_codemirror_box\', %s ); } );',
'jQuery( function() { window.%s_wp_editor = wp.codeEditor.initialize( \'%s\', %s ); } );',
$id,
$id,
wp_json_encode( $settings )
)
);
Expand Down
253 changes: 253 additions & 0 deletions classes/helpers/FrmCssScopeHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
<?php
if ( ! defined( 'ABSPATH' ) ) {
die( 'You are not allowed to call this page directly.' );
}

class FrmCssScopeHelper {

/**
* Nest the CSS.
* This function nests the CSS by adding the class name prefix to the selectors.
*
* @param string $css
* @param string $class_name
* @return string
*/
public function nest( $css, $class_name ) {
// Remove CSS comments but preserve newlines
$css = preg_replace( '/\/\*.*?\*\//s', '', $css );

$output = array();
$css = trim( $css );
$length = strlen( $css );
$i = 0;
$buffer = '';

while ( $i < $length ) {
$char = $css[ $i ];

if ( '@' === $char ) {
$brace_pos = strpos( $css, '{', $i );
if ( false === $brace_pos ) {
$buffer .= $char;
++$i;
continue;
}

$rule = substr( $css, $i, $brace_pos - $i );
$closing_brace = $this->find_matching_brace( $css, $brace_pos );
$inner_content = substr( $css, $brace_pos + 1, $closing_brace - $brace_pos - 1 );

// Don't nest keyframes content
if ( strpos( $rule, '@keyframes' ) !== false ) {
$output[] = "\n" . $rule . ' {' . $inner_content . '}' . "\n";
} else {
$output[] = "\n" . $rule . ' {';
$output[] = $this->nest( $inner_content, $class_name );
$output[] = '}' . "\n";
}

$i = $closing_brace + 1;
$buffer = '';
continue;
}//end if
Copy link
Contributor

@Crabcyborg Crabcyborg Nov 26, 2025

Choose a reason for hiding this comment

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

@Liviu-p Can we break this up a bit?

Instead of requiring a //end if, I think we could add handle_at_char/handle_open_curly_brace functions.


if ( '{' === $char ) {
$selector = trim( $buffer );
$closing_brace = $this->find_matching_brace( $css, $i );
$declarations = substr( $css, $i + 1, $closing_brace - $i - 1 );

// Preserve indentation and formatting of declarations
$declarations = $this->preserve_declaration_formatting( $declarations );

if ( '' !== $selector && '' !== trim( $declarations ) ) {
// Handle multiple selectors
$selectors = array_map( 'trim', explode( ',', $selector ) );
$prefixed_selectors = array();

foreach ( $selectors as $single_selector ) {
if ( '' !== $single_selector ) {
$prefixed_selectors[] = '.' . $class_name . ' ' . $single_selector;
}
}

if ( ! empty( $prefixed_selectors ) ) {
$output[] = "\n" . implode( ',' . "\n", $prefixed_selectors ) . ' {' . $declarations . '}' . "\n";
}
}

$i = $closing_brace + 1;
$buffer = '';
continue;
}//end if

$buffer .= $char;
++$i;
}//end while

return implode( '', $output );
}

/**
* Unnest the CSS.
* This function unnests the CSS by removing the class name prefix from the selectors.
*
* @param string $css
* @param string $class_name
* @return string
*/
public function unnest( $css, $class_name ) {
// Remove CSS comments but preserve newlines
$css = preg_replace( '/\/\*.*?\*\//s', '', $css );

$output = array();
$css = trim( $css );
$length = strlen( $css );
$i = 0;
$buffer = '';
$prefix = '.' . $class_name . ' ';
$prefix_length = strlen( $prefix );

while ( $i < $length ) {
$char = $css[ $i ];

if ( '@' === $char ) {
$brace_pos = strpos( $css, '{', $i );
if ( false === $brace_pos ) {
$buffer .= $char;
++$i;
continue;
}

$rule = substr( $css, $i, $brace_pos - $i );
$closing_brace = $this->find_matching_brace( $css, $brace_pos );
$inner_content = substr( $css, $brace_pos + 1, $closing_brace - $brace_pos - 1 );

$output[] = "\n" . $rule . ' {';
$output[] = $this->unnest( $inner_content, $class_name );
$output[] = '}' . "\n";

$i = $closing_brace + 1;
$buffer = '';
continue;
}

if ( '{' === $char ) {
$selector = trim( $buffer );
$closing_brace = $this->find_matching_brace( $css, $i );
$declarations = substr( $css, $i + 1, $closing_brace - $i - 1 );

// Preserve indentation and formatting of declarations
$declarations = $this->preserve_declaration_formatting( $declarations );

if ( '' !== $selector && '' !== trim( $declarations ) ) {
// Handle multiple selectors
$selectors = array_filter(
array_map( 'trim', explode( ',', $selector ) ),
function ( $s ) {
return '' !== $s;
}
);
$unprefixed_selectors = array();

foreach ( $selectors as $single_selector ) {
$unprefixed_selectors[] = 0 === strpos( $single_selector, $prefix )
? trim( substr( $single_selector, $prefix_length ) )
: $single_selector;
}

if ( ! empty( $unprefixed_selectors ) ) {
$output[] = "\n" . implode( ',' . "\n", $unprefixed_selectors ) . ' {' . $declarations . '}' . "\n";
}
}

$i = $closing_brace + 1;
$buffer = '';
continue;
}//end if

$buffer .= $char;
++$i;
}//end while
return implode( '', $output );
}

/**
* Preserve declaration formatting with proper indentation.
*
* @param string $declarations
* @return string
*/
private function preserve_declaration_formatting( $declarations ) {
// Trim the entire block but keep internal structure
$declarations = trim( $declarations );

if ( '' === $declarations ) {
return '';
}

// Check if declarations are already on multiple lines
if ( strpos( $declarations, "\n" ) !== false ) {
// Already formatted - preserve it
$lines = explode( "\n", $declarations );
$formatted_lines = array();

foreach ( $lines as $line ) {
$trimmed = trim( $line );
if ( '' !== $trimmed ) {
$formatted_lines[] = "\n\t" . $trimmed;
}
}

return implode( '', $formatted_lines ) . "\n";
} else {
Copy link
Contributor

@Crabcyborg Crabcyborg Nov 26, 2025

Choose a reason for hiding this comment

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

@Liviu-p Since there is a return above this, we don't really need the else.

// Single line - add minimal formatting
return ' ' . $declarations . ' ';
}
}

/**
* Find the matching brace in the CSS.
*
* @param string $css
* @param int $open_pos
* @return int
*/
private function find_matching_brace( $css, $open_pos ) {
$level = 1;
$length = strlen( $css );
$in_string = false;
$string_char = '';

for ( $i = $open_pos + 1; $i < $length; $i++ ) {
$char = $css[ $i ];

// Handle string literals to avoid matching braces inside strings
if ( ( '"' === $char || "'" === $char ) && ( 0 === $i || '\\' !== $css[ $i - 1 ] ) ) {
if ( ! $in_string ) {
$in_string = true;
$string_char = $char;
} elseif ( $char === $string_char ) {
$in_string = false;
}
continue;
}

// Skip braces inside strings
if ( $in_string ) {
continue;
}

if ( '{' === $char ) {
++$level;
} elseif ( '}' === $char ) {
--$level;
if ( 0 === $level ) {
return $i;
}
}
}//end for

return $length - 1;
}
}//end class
13 changes: 11 additions & 2 deletions classes/models/FrmStyle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public function duplicate( $id ) {
* @return array<int|WP_Error>
*/
public function update( $id = 'default' ) {
$all_instances = $this->get_all();
$all_instances = $this->get_all();
$css_scope_helper = new FrmCssScopeHelper();

if ( ! $id ) {
$new_style = (array) $this->get_new();
Expand Down Expand Up @@ -115,6 +116,11 @@ public function update( $id = 'default' ) {
$new_instance['post_content']['custom_css'] = $custom_css;
unset( $custom_css );

if ( ! empty( $new_instance['post_content']['single_style_custom_css'] ) ) {
$css_scope = 'frm_style_' . $new_instance['post_name'];
$new_instance['post_content']['single_style_custom_css'] = $css_scope_helper->nest( $new_instance['post_content']['single_style_custom_css'], $css_scope );
}

$new_instance['post_type'] = FrmStylesController::$post_type;
$new_instance['post_status'] = 'publish';

Expand Down Expand Up @@ -255,7 +261,7 @@ public function sanitize_post_content( $settings ) {
$sanitized_settings[ $key ] = $defaults[ $key ];
}

if ( 'custom_css' !== $key ) {
if ( 'custom_css' !== $key && 'single_style_custom_css' !== $key ) {
$sanitized_settings[ $key ] = $this->strip_invalid_characters( $sanitized_settings[ $key ] );
}
}
Expand Down Expand Up @@ -749,6 +755,9 @@ public function get_defaults() {
'use_base_font_size' => false,
'base_font_size' => '15px',
'field_shape_type' => 'rounded-corner',

'enable_style_custom_css' => false,
'single_style_custom_css' => '',
);

return apply_filters( 'frm_default_style_settings', $defaults );
Expand Down
Loading
Loading