Fixed echo in five files#275
Conversation
WalkthroughFive view templates were changed to actually render previously prepared/sanitized strings by adding Changes
Sequence Diagram(s)sequenceDiagram
participant Template as View Template
participant Sanitizer as wp_kses / wp_kses_post / esc_html
participant Browser as Browser / Email
Note over Template,Sanitizer: Previous flow (prepared but not output)
Template->>Sanitizer: prepare sanitized/escaped string
Sanitizer-->>Template: sanitized string (returned)
Note right of Template: No echo — nothing sent to Browser/Email
rect rgba(100,200,120,0.12)
Note over Template,Sanitizer: New flow (with echo)
Template->>Sanitizer: prepare sanitized/escaped string
Sanitizer-->>Template: sanitized string
Template->>Browser: echo sanitized string (rendered HTML/text)
Template->>Email: echo sanitized string (rendered in email)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
views/events/widget-message.php(1 hunks)views/wizards/host-integrations/cloudflare-instructions.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-11T20:16:49.880Z
Learnt from: superdav42
Repo: Multisite-Ultimate/ultimate-multisite PR: 256
File: inc/managers/class-membership-manager.php:140-142
Timestamp: 2025-11-11T20:16:49.880Z
Learning: The wu_log_add() function in WP Ultimo accepts both string and \WP_Error for the $message parameter (second parameter). It's valid to pass WP_Error objects directly to wu_log_add() without extracting the error message first, as the Logger::add() method handles both types.
Applied to files:
views/events/widget-message.php
🧬 Code graph analysis (1)
views/events/widget-message.php (2)
inc/models/class-event.php (1)
get_message(255-260)inc/functions/helper.php (1)
wu_kses_allowed_html(326-589)
⏰ 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). (4)
- GitHub Check: PHP 8.4
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
|
|
||
| <span class="wu-my-1 wu-inline-block"> | ||
| <?php wp_kses($object->get_message(), wu_kses_allowed_html()); ?> | ||
| <?php echo wp_kses($object->get_message(), wu_kses_allowed_html()); ?> |
There was a problem hiding this comment.
Critical bug fix - event message is now properly displayed.
The addition of echo ensures the sanitized event message is actually rendered in the widget. Previously, the message was sanitized but never displayed, making this widget non-functional for its intended purpose.
🤖 Prompt for AI Agents
In views/events/widget-message.php around line 16, the sanitized event message
was not being output so the widget showed no message; fix by echoing the
sanitized result returned by $object->get_message() (i.e., ensure the call to
wp_kses(...) is echoed) so the sanitized content is rendered in the widget.
| <strong><?php esc_html_e('Before we start...', 'ultimate-multisite'); ?></strong><br> | ||
| <?php // translators: %s the url ?> | ||
| <?php wp_kses_post(sprintf(__('This integration is really aimed at people that do not have access to an Enterprise Cloudflare account, since that particular tier supports proxying on wildcard DNS entries, which makes adding each subdomain unecessary. If you own an enterprise tier account, you can simply follow <a class="wu-no-underline" href="%s" target="_blank">this tutorial</a> to create the wildcard entry and deactivate this integration entirely.', 'ultimate-multisite'), 'https://support.cloudflare.com/hc/en-us/articles/200169356-How-do-I-use-WordPress-Multi-Site-WPMU-With-Cloudflare')); ?> | ||
| <?php echo wp_kses_post(sprintf(__('This integration is really aimed at people that do not have access to an Enterprise Cloudflare account, since that particular tier supports proxying on wildcard DNS entries, which makes adding each subdomain unecessary. If you own an enterprise tier account, you can simply follow <a class="wu-no-underline" href="%s" target="_blank">this tutorial</a> to create the wildcard entry and deactivate this integration entirely.', 'ultimate-multisite'), 'https://support.cloudflare.com/hc/en-us/articles/200169356-How-do-I-use-WordPress-Multi-Site-WPMU-With-Cloudflare')); ?> |
There was a problem hiding this comment.
Critical bug fix - content is now properly displayed.
The addition of echo ensures the sanitized HTML notice is actually rendered on the page. Previously, wp_kses_post() was called but its return value was discarded, making this important preface message invisible to users.
🤖 Prompt for AI Agents
In views/wizards/host-integrations/cloudflare-instructions.php around line 21,
the call to wp_kses_post(...) was returning sanitized HTML but its result was
not echoed, so the message never appeared; fix it by outputting the sanitized
string (prepend echo) so the formatted translated message is rendered to the
page, keeping the existing sprintf(...) parameters and escaping function
unchanged.
🧩 Analysis chain
Verify similar patterns in other view files.
Since this was a missing echo statement, check if other view templates have the same issue with sanitization functions being called without outputting their results.
🏁 Script executed:
#!/bin/bash
# Description: Search for potential missing echo statements in view files
# Look for wp_kses, wp_kses_post, esc_html, esc_attr calls within PHP tags that lack echo
# Search for wp_kses* calls in view files that might be missing echo
echo "=== Checking for wp_kses calls without echo in PHP view files ==="
rg -n -t php -g 'views/**/*.php' '^\s*<\?php\s+wp_kses(?!.*echo)' -C 2
echo -e "\n=== Checking for wp_kses_post calls without echo in PHP view files ==="
rg -n -t php -g 'views/**/*.php' '^\s*<\?php\s+wp_kses_post\(' -C 2
echo -e "\n=== Checking for sanitization function patterns that might need echo ==="
rg -n -t php -g 'views/**/*.php' -P '^\s*<\?php\s+(?!echo|if|while|for|foreach|switch|return|\$).*(?:wp_kses|esc_html|esc_attr|sanitize_)' -C 2Length of output: 122403
Add missing echo statement and fix similar issue in related file.
The original issue in cloudflare-instructions.php:21 exists as flagged. A similar pattern was found in another integration file that needs correction:
views/wizards/host-integrations/ready.php:18— Missingechobeforeesc_html(sprintf(...)). Should be:<?php echo esc_html(sprintf(...))
Both issues follow the same pattern: non-output-performing sanitization functions (esc_html(), wp_kses_post()) require an explicit echo statement. Functions ending in _e() (e.g., esc_html_e()) handle output automatically.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
views/wizards/host-integrations/ready.php (1)
18-18: LGTM! The echo fix correctly renders the message.The addition of
echoproperly fixes the bug where the sanitized and translated string was not being output. The use ofesc_html()is appropriate for plain-text content, and the i18n pattern withsprintf()is correct.Optional: Consider adding a defensive check for the
$integrationvariable.While WordPress view templates typically receive variables from controllers, adding a check would make the code more resilient:
<p class="wu-text-lg wu-text-gray-600 wu-my-4"> <?php // translators: % site title ?> - <?php echo esc_html(sprintf(__('The integration with %s was correctly setup and is now ready! Now, every time a new domain is added to your platform, Ultimate Multisite will sync that with your application automatically.', 'ultimate-multisite'), $integration->get_title())); ?> + <?php echo esc_html(sprintf(__('The integration with %s was correctly setup and is now ready! Now, every time a new domain is added to your platform, Ultimate Multisite will sync that with your application automatically.', 'ultimate-multisite'), isset($integration) ? $integration->get_title() : '')); ?> </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
views/wizards/host-integrations/ready.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). (2)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
|
Hello @superdav42 When you can, could you take a look at this PR of mine? Thanks. |
|
@DAnn2012 Thanks for the PR! it was spot on. |
2 participants
Fixed echo in five files
Summary by CodeRabbit
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.