-
Notifications
You must be signed in to change notification settings - Fork 67
fix: use wp_kses_post instead of esc_html to allow dynamic content #3620
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.
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
Conversation
WalkthroughReplaces esc_html(...) with wp_kses_post(...) for the Read More link text in the Posts block rendering, enabling sanitized HTML output in that specific field. No other logic or public interfaces are changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
🤖 Pull request artifacts
|
There was a problem hiding this 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
📒 Files selected for processing (1)
plugin.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: build
plugin.php
Outdated
|
|
||
| defined( 'STACKABLE_SHOW_PRO_NOTICES' ) || define( 'STACKABLE_SHOW_PRO_NOTICES', true ); | ||
| defined( 'STACKABLE_BUILD' ) || define( 'STACKABLE_BUILD', 'free' ); | ||
| defined( 'STACKABLE_BUILD' ) || define( 'STACKABLE_BUILD', 'premium' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRITICAL: Wrong code change - doesn't match PR objectives.
The PR title and description state this should fix dynamic content rendering in the Read More Link by changing esc_html to wp_kses_post, but this change modifies the plugin build type from 'free' to 'premium'. This is a completely unrelated change that would:
- Enable Freemius SDK loading (line 257-259)
- Attempt to load premium-only features (line 324-332)
- Fundamentally alter the plugin's initialization and feature set
- Not address issue Posts block – Dynamic content in Read More Link displays tags and incorrect content #3618 at all
This appears to be either:
- Code committed to the wrong branch
- An accidental change that wasn't intended
- A completely incorrect PR description
The actual fix for issue #3618 should be in the Posts block rendering code (likely in src/block/posts/ directory) where the Read More Link is output, not in the main plugin initialization file.
6e6142a to
28f870a
Compare
There was a problem hiding this 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)
src/block/posts/index.php (1)
57-61: Consider reviewing sanitization consistency for title replacement.While reviewing the sanitization approach in this file, I noticed that the title replacement (line 61) doesn't apply any sanitization function before inserting
$titleinto the template. For consistency and defense-in-depth, consider whetherwp_kses_post()oresc_html()should be applied here as well, depending on whether titles should support HTML formatting.Note: This is a broader consistency observation and not directly related to the current fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/block/posts/index.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
🔇 Additional comments (1)
src/block/posts/index.php (1)
135-135: Approve change: use wp_kses_post for Read More link
Usingwp_kses_post()instead ofesc_html()correctly allows safe HTML (e.g.<span>) in the Read More link while preserving sanitization and matching the excerpt handling.
fixes #3618
Summary by CodeRabbit