-
-
Notifications
You must be signed in to change notification settings - Fork 64
code changes for wp.org review #183
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
WalkthroughA large refactor switches many UI components from returning HTML strings to printing output directly, updates tooltip handling to printing or ARIA attributes, tightens escaping (wp_kses/esc_html), removes legacy checkout flows, adds invoice CSS assets, and adjusts various inline-script and error-handling patterns. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin UI
participant Page as Checkout_Form_Edit_Admin_Page
participant WP as WP Scripts/Styles
participant T as Template Engine
Admin->>Page: Open "Checkout Form Preview"
Page->>WP: wp_enqueue_scripts()
Page->>WP: wp_print_head_scripts()
Note over Page,T: Streaming render (no buffering)
loop For each step
Page->>T: wu_get_template('checkout/form', step data)
T-->>Admin: Step HTML output
Page-->>Admin: "----- Step Separator -----"
end
Page->>WP: wp_print_footer_scripts()
Page-->>Admin: exit
sequenceDiagram
autonumber
actor Caller
participant BE as UI\Base_Element
participant El as Concrete Element
participant T as Template Engine
Caller->>BE: display($atts)
BE->>El: output($atts) (void)
El->>T: wu_get_template(view, $atts)
T-->>Caller: HTML printed directly
Note over BE,El: Methods no longer return strings
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (12)
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 |
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
inc/admin-pages/class-broadcast-edit-admin-page.php (2)
188-206: Broken control flow: switch/case compares array to booleans — replace with if/elseif on $targets_countThe switch uses
$targetsas the subject and boolean expressions as cases, which will never match as intended. This breaks the “No customer found” and multi-customer branches.Apply this refactor to make the logic correct and self-evident:
- switch ($targets) { - case $targets_count < 0: + if ($targets_count <= 0) { printf( "<span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> <div class=''> <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> </div>", esc_html__('No customer found', 'multisite-ultimate') ); - - break; - case 1 === $targets_count: + } elseif (1 === $targets_count) { $customer = wu_get_customer($targets[0]); @@ - break; - case $targets_count > 1: + } else { // $targets_count > 1 foreach ($targets as $key => $target) { $customer = wu_get_customer($target); @@ - } - - break; - } + } + }Also applies to: 240-313
226-238: Incorrect printf argument order — avatar/name swappedThe placeholders expect: link, avatar, display_name, id, email. Current argument order outputs the display name where the avatar should be and the avatar HTML where the name should be.
- printf( + printf( '<a href="%s" class="wu-p-1 wu-flex wu-flex-grow wu-rounded wu-items-center wu-no-underline"> %s <div class="wu-pl-2"> <strong class="wu-block">%s <small class="wu-font-normal">(#%s)</small></strong> <small>%s</small> </div> </a>', - esc_attr($customer_link), - esc_html($display_name), - $avatar, // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - esc_html($id), - esc_html($email) + esc_attr($customer_link), + $avatar, // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + esc_html($display_name), + esc_html($id), + esc_html($email) );inc/checkout/class-legacy-checkout.php (1)
669-677: Missingcreate_account()Handler in Legacy_CheckoutThe
create-accountstep in inc/checkout/class-legacy-checkout.php (around lines 669–677) references[$this, 'create_account'], but nocreate_account()method exists in this class, its traits, or elsewhere in the codebase. This will trigger a fatal error when the step runs.• File: inc/checkout/class-legacy-checkout.php (lines 669–677)
• Issue:create_account()is not defined in Legacy_Checkout or any imported trait/functionNext steps—address this critical breakage before merging:
- Implement a
public function create_account( … )in Legacy_Checkout.- OR, if there’s an existing handler method elsewhere, update the step’s
handlerto point to the correct method.- OR, remove (or comment out) the
create-accountstep registration until the handler is in place.
🧹 Nitpick comments (5)
inc/admin-pages/class-webhook-list-admin-page.php (1)
181-192: Capability enforcement confirmed; optional sanitization and validation hardening recommendedI’ve confirmed that submissions to the
add_new_webhook_modalform are gated by both a nonce and the declaredwu_edit_webhookscapability upstream inForm_Manager::handle_form():
- In
Form_Manager::security_checks()(inc/managers/class-form-manager.php line 183–188), the code callsensuring that only users withif ( ! current_user_can( $form['capability'] ) ) { $this->display_form_unavailable(); }wu_edit_webhookscan proceed.- In
Form_Manager::handle_form()(inc/managers/class-form-manager.php line 149–157), the code callsenforcing the expected nonce for this form.if ( ! wp_verify_nonce( wu_request('_wpnonce'), 'wu_form_' . $form['id'] ) ) { wp_send_json_error(); }
With both checks in place, the pathway is secure against unauthorized access.As an optional hardening step, you may tighten sanitization/validation in
handle_add_new_webhook_modal()(inc/admin-pages/class-webhook-list-admin-page.php lines 181–192) with something like:- // Nonce check handled in Form_Manager::handle_form. + // Nonce and capability enforced upstream; add event slug validation and stricter sanitization. - $status = wu_create_webhook( + // Validate 'event' against registered slugs. + $__event = ! empty( $_POST['event'] ) ? sanitize_text_field( wp_unslash( $_POST['event'] ) ) : false; // phpcs:ignore WordPress.Security.NonceVerification + $__allowed_events = array_keys( (array) wu_get_event_types() ); + if ( $__event && ! in_array( $__event, $__allowed_events, true ) ) { + wp_send_json_error( new \WP_Error( 'invalid_event', __( 'Invalid event.', 'multisite-ultimate' ) ) ); + } + + $status = wu_create_webhook( [ 'name' => ! empty( $_POST['name'] ) ? sanitize_text_field( wp_unslash( $_POST['name'] ) ) : false, // phpcs:ignore WordPress.Security.NonceVerification - 'webhook_url' => ! empty($_POST['webhook_url']) ? sanitize_url(wp_unslash($_POST['webhook_url'])) : false, // phpcs:ignore WordPress.Security.NonceVerification - 'event' => $__event, - 'active' => ! empty($_POST['active']) ? sanitize_text_field(wp_unslash($_POST['active'])) : false, // phpcs:ignore WordPress.Security.NonceVerification + 'webhook_url' => ! empty( $_POST['webhook_url'] ) ? esc_url_raw( wp_unslash( $_POST['webhook_url'] ) ) : false, // phpcs:ignore WordPress.Security.NonceVerification + 'event' => $__event, + 'active' => ! empty( $_POST['active'] ) ? 1 : 0, // phpcs:ignore WordPress.Security.NonceVerification 'event_count' => ! empty( $_POST['event_count'] ) ? (int) $_POST['event_count'] : 0, // phpcs:ignore WordPress.Security.NonceVerification 'date_created' => ! empty( $_POST['date_created'] ) ? sanitize_text_field( wp_unslash( $_POST['date_created'] ) ) : wu_get_current_time( 'mysql', true ), // phpcs:ignore WordPress.Security.NonceVerification 'date_modified' => ! empty( $_POST['date_modified'] ) ? sanitize_text_field( wp_unslash( $_POST['date_modified'] ) ) : wu_get_current_time( 'mysql', true ), // phpcs:ignore WordPress.Security.NonceVerification - 'migrated_from_id' => ! empty($_POST['migrated_from_id']) ? (int) $_POST['migrated_from_id'] : 0, // phpcs:ignore WordPress.Security.NonceVerification + 'migrated_from_id' => ! empty( $_POST['migrated_from_id'] ) ? absint( $_POST['migrated_from_id'] ) : 0, // phpcs:ignore WordPress.Security.NonceVerification ] );
- No further capability checks are needed here since
register_form()already set'capability' => 'wu_edit_webhooks'.- The above diff is purely optional and intended to tighten data validation before persisting the webhook.
inc/admin-pages/class-broadcast-edit-admin-page.php (2)
462-467: Grammar and sanitization: fix comment and prefer absint() on idSmall polish and safety tweak: correct the comment grammar and use absint() instead of (int) for the query param.
- // Data is only being fetch, nothing is being modified, no need for nonce check. + // Data is only being fetched; nothing is being modified, so no nonce check is required. - $item = $query->get_item_by('id', (int) $_GET['id']); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $item = $query->get_item_by('id', absint($_GET['id'])); // phpcs:ignore WordPress.Security.NonceVerification.Recommended
491-494: Typo in docblock: “Wether” → “Whether”Minor docblock typo.
- * Wether or not this pages should have an editor field. + * Whether or not this page should have an editor field.views/invoice/template.php (1)
11-13: Ensure stylesheet actually prints in all render contexts (enqueue or fallback).
$wp_styles->do_item('wu-invoice')only outputs if the handle is registered/enqueued. In non-standard render paths (e.g., admin-ajax, PDF render), the handle might not be queued yet. Consider enqueuing as a fallback beforedo_item()to avoid missing styles.- $wp_styles = wp_styles(); - - $wp_styles->do_item('wu-invoice'); + $wp_styles = wp_styles(); + if ( ! wp_style_is('wu-invoice', 'enqueued') && ! wp_style_is('wu-invoice', 'done') ) { + // Fallback enqueue in case upstream missed it (e.g., direct template rendering/PDF contexts). + wp_enqueue_style('wu-invoice', wu_get_asset('invoice.css', 'css'), [], \WP_Ultimo::VERSION); + } + $wp_styles->do_item('wu-invoice');Would you confirm that
inc/invoices/class-invoice.php::render()is always invoked before this template on every code path (including the customizer preview via admin-ajax), so the handle is enqueued? If not, the fallback above will cover it.inc/managers/class-site-manager.php (1)
495-505: Inline style depends on 'login' handle; enqueue explicitly to guarantee output.
wp_add_inline_style('login', ...)returns false if the handle isn’t enqueued/registered yet. On some setups, running at priority 10 can race core. Either enqueue the handle here or bump the hook priority when addingcustom_login_logo.- wp_add_inline_style( + // Ensure the 'login' style handle exists/enqueued before attaching inline CSS. + wp_enqueue_style('login'); + wp_add_inline_style( 'login', - sprintf( - '#login h1 a, .login h1 a { - background-image: url(%s); - background-position: center center; - background-size: contain; - }', - esc_url($logo) - ) + sprintf( + '#login h1 a, .login h1 a {' . "\n" . + ' background-image: url("%s");' . "\n" . + ' background-position: center center;' . "\n" . + ' background-size: contain;' . "\n" . + '}', + esc_url($logo) + ) );Optionally, register the action with priority 11:
add_action('login_enqueue_scripts', [$this, 'custom_login_logo'], 11);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
inc/admin-pages/class-broadcast-edit-admin-page.php(1 hunks)inc/admin-pages/class-hosting-integration-wizard-admin-page.php(1 hunks)inc/admin-pages/class-webhook-list-admin-page.php(1 hunks)inc/checkout/class-legacy-checkout.php(3 hunks)inc/functions/legacy.php(2 hunks)inc/invoices/class-invoice.php(2 hunks)inc/limitations/class-limit-site-templates.php(1 hunks)inc/limits/class-theme-limits.php(1 hunks)inc/managers/class-site-manager.php(1 hunks)views/invoice/template.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
inc/invoices/class-invoice.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-910)inc/admin-pages/class-invoice-template-customize-admin-page.php (1)
generate_invoice_preview(95-113)
inc/admin-pages/class-hosting-integration-wizard-admin-page.php (1)
inc/functions/helper.php (1)
wu_request(132-137)
inc/limitations/class-limit-site-templates.php (1)
inc/admin-pages/class-edit-admin-page.php (1)
process_save(216-244)
views/invoice/template.php (1)
inc/admin-pages/class-invoice-template-customize-admin-page.php (3)
Invoice_Template_Customize_Admin_Page(20-364)init(81-87)generate_invoice_preview(95-113)
inc/functions/legacy.php (1)
inc/admin-pages/class-email-edit-admin-page.php (1)
register_scripts(108-114)
⏰ 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). (12)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.0, chrome)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.0, firefox)
🔇 Additional comments (6)
inc/limitations/class-limit-site-templates.php (1)
239-245: LGTM: Clarified nonce responsibility and now returning the updated moduleComment clarifies upstream nonce handling and returning $module aligns with the docblock and expected API. Good cleanup.
inc/admin-pages/class-hosting-integration-wizard-admin-page.php (1)
243-244: ✅ Passing Sanitized Request Data Is Safe HereThe configuration-results template only uses the passed
$postin<?php echo esc_html( $integration->get_constants_string( $post ) ); ?>It does not parse or decode a raw JSON string—rather, it expects an array (as supplied by
wu_request('post')) and outputs an HTML-escaped string of constants. No JSON handling (quotes, braces, etc.) takes place in the view, so thestripslashes_deepsanitization won’t break anything.• If you ever need to consume true JSON in a template, you can still decode it explicitly (e.g.
json_decode((string) wu_request('post'), true)).
• In this case, however, leave the code as-is:'post' => wu_request('post'),No further changes required.
inc/invoices/class-invoice.php (2)
171-172: Enqueueing invoice stylesheet in render() — good alignment with template do_item()This keeps styling centralized and consistent with views/invoice/template.php using do_item('wu-invoice'). Looks good.
192-196: Constant Usage Verified – No Further Action RequiredA repository-wide search confirmed that
WU_GENERATING_PDFis only defined ininc/invoices/class-invoice.php(lines 192–196) and is not referenced elsewhere. No downstream branches or side effects exist for this constant.• inc/invoices/class-invoice.php:192–196 —
define('WU_GENERATING_PDF', true)Optional: If your hosting environment restricts external HTTP requests, you may inline critical CSS when generating the PDF:
$css_url = wu_get_asset('invoice.css', 'css'); $css = wp_remote_retrieve_body(wp_remote_get($css_url)); $this->printer->WriteHTML($css, \Mpdf\HTMLParserMode::HEADER_CSS); $this->printer->WriteHTML($this->render(), \Mpdf\HTMLParserMode::HTML_BODY);inc/checkout/class-legacy-checkout.php (2)
421-423: Plan/Domain/begin-signup handlers commented out — confirm intended UX and data flow.Commenting the handlers means those steps won’t persist or validate input unless another step’s handler covers it. Ensure:
- Validation/persistence for plan and domain fields is performed elsewhere (e.g., within
account_saveorcreate_account).- Any hooks relying on the old handlers are updated.
Would you like a quick audit script to grep for references to
plans_save,domain_save, andbegin_signupto ensure there are no dangling hooks?Also applies to: 447-449, 653-657
676-676: No change needed – plugin already requires PHP 7.4+The WordPress plugin header (
readme.txtline 7 andmultisite-ultimate.phpline 16) specifies “Requires PHP: 7.4.30,” so the use of the1_000_000_000numeric separator (introduced in PHP 7.4) is fully supported and will not cause parse errors on installs. There’s no need to replace it with1000000000.
|
|
||
| $script_data = [ | ||
| 'requires' => $field['requires'], | ||
| 'field_slug' => $field_slug, | ||
| 'elements' => $elements | ||
| ]; | ||
|
|
||
| wp_add_inline_script('jquery', sprintf(' | ||
| document.addEventListener("DOMContentLoaded", function() { | ||
| var requires = %s, | ||
| target_field = document.getElementById("%s-field"); | ||
| var display_field = function(target_field, requires, velocity) { | ||
| var conditions_count = Object.keys(requires).length, | ||
| conditions_met = 0; | ||
| requires.forEach(function(element, value) { | ||
|
|
||
| var element = document.getElementById(element), | ||
| element_value = element.value; | ||
| element_value = element.value; | ||
| if (element.type === "checkbox") { | ||
| var is_checked = element.checked; | ||
| if (is_checked === value) { | ||
| conditions_met++; | ||
| } | ||
| return true; | ||
| } | ||
| value = Array.isArray(value) ? value : [value]; | ||
| if (value.indexOf(element_value) > -1) { | ||
| conditions_met++; | ||
| } | ||
| }); | ||
| var is_checked = element.checked; | ||
|
|
||
| if (is_checked === value) { | ||
| conditions_met++; | ||
| } | ||
|
|
||
| return true; | ||
|
|
||
| } | ||
|
|
||
| value = Array.isArray(value) ? value : [value]; | ||
|
|
||
| if (value.indexOf(element_value) > -1) { | ||
|
|
||
| conditions_met++; | ||
|
|
||
| } | ||
|
|
||
| }); | ||
|
|
||
| if (conditions_met === conditions_count) { | ||
|
|
||
| target_field.slideDown(velocity); | ||
|
|
||
| } else { | ||
|
|
||
| target_field.slideUp(velocity); | ||
|
|
||
| } // end | ||
|
|
||
| } | ||
| if (conditions_met === conditions_count) { | ||
| target_field.slideDown(velocity); | ||
| } else { | ||
| target_field.slideUp(velocity); | ||
| } | ||
| } | ||
| display_field(target_field, requires, 0); | ||
| display_field(target_field, requires, 0); | ||
| [<?php echo $elements; ?>].forEach(function(element) { | ||
| document.getElementById(element).addEventListener('change', function() { | ||
| [%s].forEach(function(element) { | ||
| document.getElementById(element).addEventListener("change", function() { | ||
| display_field(target_field, requires, 300); | ||
| }); | ||
| }); | ||
|
|
||
| }); | ||
| </script> | ||
|
|
||
| <?php | ||
| });', | ||
| wp_json_encode($script_data['requires']), | ||
| esc_js($script_data['field_slug']), | ||
| $script_data['elements'] | ||
| ), 'after'); | ||
|
|
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.
Fix broken requires-logic script: malformed element list, object iteration, and jQuery misuse.
Current issues:
requiresis an object; callingforEachon it will throw.$elementsbecomes an unquoted comma list, yielding invalid JS ([id1, id2]).slideUp/slideDowncalled on a DOM element instead of a jQuery object.
Apply this patch to generate valid JS and use jQuery for the animations:
- wp_enqueue_script('jquery');
-
- $script_data = [
- 'requires' => $field['requires'],
- 'field_slug' => $field_slug,
- 'elements' => $elements
- ];
-
- wp_add_inline_script('jquery', sprintf('
- document.addEventListener("DOMContentLoaded", function() {
- var requires = %s,
- target_field = document.getElementById("%s-field");
-
- var display_field = function(target_field, requires, velocity) {
- var conditions_count = Object.keys(requires).length,
- conditions_met = 0;
-
- requires.forEach(function(element, value) {
- var element = document.getElementById(element),
- element_value = element.value;
-
- if (element.type === "checkbox") {
- var is_checked = element.checked;
- if (is_checked === value) {
- conditions_met++;
- }
- return true;
- }
-
- value = Array.isArray(value) ? value : [value];
- if (value.indexOf(element_value) > -1) {
- conditions_met++;
- }
- });
-
- if (conditions_met === conditions_count) {
- target_field.slideDown(velocity);
- } else {
- target_field.slideUp(velocity);
- }
- }
-
- display_field(target_field, requires, 0);
-
- [%s].forEach(function(element) {
- document.getElementById(element).addEventListener("change", function() {
- display_field(target_field, requires, 300);
- });
- });
- });',
- wp_json_encode($script_data['requires']),
- esc_js($script_data['field_slug']),
- $script_data['elements']
- ), 'after');
+ wp_enqueue_script('jquery');
+
+ $requires = $field['requires'];
+ $element_ids = array_keys($requires);
+
+ wp_add_inline_script(
+ 'jquery',
+ sprintf(
+ 'document.addEventListener("DOMContentLoaded", function(){' .
+ 'var requires = %s;' .
+ 'var $target = jQuery("#%s-field");' .
+ 'function evalVisibility(velocity){' .
+ 'var entries = Object.entries(requires);' .
+ 'var met = 0;' .
+ 'entries.forEach(function(pair){' .
+ 'var id = pair[0], expected = pair[1];' .
+ 'var el = document.getElementById(id);' .
+ 'if (!el) { return; }' .
+ 'if (el.type === "checkbox") {' .
+ 'if (!!el.checked === !!expected) { met++; }' .
+ '} else {' .
+ 'var expectedArr = Array.isArray(expected) ? expected : [expected];' .
+ 'if (expectedArr.indexOf(el.value) > -1) { met++; }' .
+ '}' .
+ '});' .
+ 'if (met === entries.length) {' .
+ '$target.stop(true, true).slideDown(velocity);' .
+ '} else {' .
+ '$target.stop(true, true).slideUp(velocity);' .
+ '}' .
+ '}' .
+ 'evalVisibility(0);' .
+ '%s.forEach(function(id){' .
+ 'var el = document.getElementById(id);' .
+ 'if (!el) { return; }' .
+ 'el.addEventListener("change", function(){ evalVisibility(300); });' .
+ 'el.addEventListener("input", function(){ evalVisibility(300); });' .
+ '});' .
+ '});',
+ wp_json_encode($requires),
+ esc_js($field_slug),
+ wp_json_encode($element_ids)
+ ),
+ 'after'
+ );🤖 Prompt for AI Agents
In inc/functions/legacy.php around lines 146-199, the inline script generates
invalid JS and misuses DOM APIs: iterate over the requires object with
Object.keys/forEach (or Object.entries) instead of requires.forEach; JSON-encode
the $elements array (use wp_json_encode($script_data['elements'])) so the
generated list is a proper quoted JS array; and use jQuery for animations and
value/checked checks (e.g. set target_field as a jQuery object like
jQuery('#<field_slug>-field') and lookup element values via jQuery selectors,
calling .slideUp/.slideDown on the jQuery object). Ensure you also adapt the
condition-check logic to read element values/checked through jQuery and compare
against the requires values when iterating keys.
| <?php | ||
| wp_add_inline_script('jquery', sprintf(' | ||
| document.addEventListener("DOMContentLoaded", function() { | ||
| var input = document.getElementById("%s"); | ||
| input.addEventListener("keyup", function() { | ||
| wu_check_pass_strength("#%s", "#%s"); | ||
| }); | ||
| }); | ||
| </script> | ||
| });', | ||
| esc_js($field_slug), | ||
| esc_js($field_slug), | ||
| esc_js($field_slug) | ||
| ), 'after'); | ||
| ?> |
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.
Password strength checker targets the wrong element.
The second argument to wu_check_pass_strength() should be the strength meter element (#pass-strength-result), not the input field again.
- wp_add_inline_script('jquery', sprintf('
+ wp_add_inline_script('jquery', sprintf('
document.addEventListener("DOMContentLoaded", function() {
var input = document.getElementById("%s");
input.addEventListener("keyup", function() {
- wu_check_pass_strength("#%s", "#%s");
+ wu_check_pass_strength("#%s", "#pass-strength-result");
});
});',
esc_js($field_slug),
- esc_js($field_slug),
- esc_js($field_slug)
+ esc_js($field_slug)
), 'after');📝 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.
| <?php | |
| wp_add_inline_script('jquery', sprintf(' | |
| document.addEventListener("DOMContentLoaded", function() { | |
| var input = document.getElementById("%s"); | |
| input.addEventListener("keyup", function() { | |
| wu_check_pass_strength("#%s", "#%s"); | |
| }); | |
| }); | |
| </script> | |
| });', | |
| esc_js($field_slug), | |
| esc_js($field_slug), | |
| esc_js($field_slug) | |
| ), 'after'); | |
| ?> | |
| <?php | |
| wp_add_inline_script('jquery', sprintf(' | |
| document.addEventListener("DOMContentLoaded", function() { | |
| var input = document.getElementById("%s"); | |
| input.addEventListener("keyup", function() { | |
| wu_check_pass_strength("#%s", "#pass-strength-result"); | |
| }); | |
| });', | |
| esc_js($field_slug), | |
| esc_js($field_slug) | |
| ), 'after'); | |
| ?> |
🤖 Prompt for AI Agents
In inc/functions/legacy.php around lines 275 to 287, the inline script calls
wu_check_pass_strength("#%s", "#%s") but passes the input field slug twice;
update the second placeholder to target the strength meter ID instead of the
input. Replace the second esc_js($field_slug) argument with
esc_js('pass-strength-result') (so the formatted call becomes
wu_check_pass_strength("#<field_slug>", "#pass-strength-result")), ensuring the
meter selector is correct and keep the third argument as-is.
| wp_add_inline_script( | ||
| 'theme', | ||
| 'if (typeof wu_theme_settings !== "undefined") { | ||
| let content = document.getElementById("tmpl-theme").innerHTML; | ||
| content = content.replace(new RegExp("(<a class=\\"button activate\\"[^<]*<\\/a>)", "g"), "<# if ( !wu_theme_settings.themes_not_available.includes(data.id) ) { #>$1<# } else { #> {{{ wu_theme_settings.replacement_message.replace(\\"EXTENSION\\", data.id) }}} <# } #>"); | ||
| document.getElementById("tmpl-theme").innerHTML = content; | ||
| }', | ||
| 'before' | ||
| ); | ||
|
|
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.
Inline script runs before wp_localize_script('theme', ...) → condition never executes.
With wp_add_inline_script('theme', ..., 'before'), the code runs before wu_theme_settings is defined (localization prints later). The typeof wu_theme_settings !== "undefined" guard causes a no-op and the template is never patched.
Apply this to run after the handle is localized/loaded and to guard DOM availability:
- wp_add_inline_script(
- 'theme',
- 'if (typeof wu_theme_settings !== "undefined") {
- let content = document.getElementById("tmpl-theme").innerHTML;
- content = content.replace(new RegExp("(<a class=\\"button activate\\"[^<]*<\\/a>)", "g"), "<# if ( !wu_theme_settings.themes_not_available.includes(data.id) ) { #>$1<# } else { #> {{{ wu_theme_settings.replacement_message.replace(\\"EXTENSION\\", data.id) }}} <# } #>");
- document.getElementById("tmpl-theme").innerHTML = content;
- }',
- 'before'
- );
+ wp_add_inline_script(
+ 'theme',
+ '(function() {' +
+ 'function patch(){' +
+ 'var t = document.getElementById("tmpl-theme");' +
+ 'if (!t || typeof wu_theme_settings === "undefined") return;' +
+ 'var content = t.innerHTML;' +
+ 'content = content.replace(' +
+ 'new RegExp("(<a class=\\"button activate\\"[^<]*<\\/a>)","g"),' +
+ '"<# if ( !wu_theme_settings.themes_not_available.includes(data.id) ) { #>$1<# } else { #> {{{ wu_theme_settings.replacement_message.replace(\\"EXTENSION\\", data.id) }}} <# } #>"' +
+ ');' +
+ 't.innerHTML = content;' +
+ '}' +
+ 'if (document.readyState === "loading") {' +
+ 'document.addEventListener("DOMContentLoaded", patch);' +
+ '} else { patch(); }' +
+ '})();',
+ 'after'
+ );This ensures:
- Localization (
wu_theme_settings) is available. - The Underscore template element exists before mutation.
🤖 Prompt for AI Agents
In inc/limits/class-theme-limits.php around lines 177 to 186, the inline script
is added with 'before' so it runs prior to wp_localize_script('theme') and
before the template exists; change the wp_add_inline_script call to add the
script 'after' the handle is printed and update the JS to first check for both
window.wu_theme_settings and the template element (e.g., guard with if (typeof
wu_theme_settings !== "undefined" && document.getElementById('tmpl-theme')) or
wrap logic in a DOMContentLoaded/window.load callback) so the localization is
available and the DOM template exists before mutating it.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
inc/compat/class-edit-users-compat.php (4)
143-147: Fix fatal due to wrong function name (esc_Html_e → esc_html_e).WordPress escaping/translation helper names are lowercase. Using esc_Html_e() will trigger a fatal “undefined function” on this screen.
Apply:
-<th scope="row"><?php esc_Html_e('Skip Confirmation Email', 'multisite-ultimate'); ?></th> +<th scope="row"><?php esc_html_e('Skip Confirmation Email', 'multisite-ultimate'); ?></th>
129-151: Only render the checkbox on the ‘add-existing-user’ form.Your comment states “Only show for existing users form,” but the code doesn’t gate by context. The user_new_form hook passes a $context of 'add-existing-user' (Multisite) or 'add-new-user' (single/network). Add a guard to avoid showing this on the wrong form. (wpseek.com)
Suggested patch:
public function add_noconfirmation_field($context) { // Only show for existing users form and only for site admins (not network admins) + if ('add-existing-user' !== $context) { + return; + } if (current_user_can('manage_network_users')) { return; }
35-37: Don’t hook createuser; limit scope to adduser to reduce privilege surface.Granting temporary manage_network_users during createuser isn’t required for the “skip confirmation” behavior and widens the escalation window unnecessarily. Keeping it on adduser only is sufficient for the add-existing-user flow.
Apply:
- add_action('admin_action_adduser', [$this, 'handle_adduser_action'], 5); - add_action('admin_action_createuser', [$this, 'handle_adduser_action'], 5); + add_action('admin_action_adduser', [$this, 'handle_adduser_action'], 5);
159-168: Gate temporary capability by the checkbox and nonce; otherwise this escalates on every add-user request.Current code adds user_has_cap for any visit to user-new.php by a site admin, even when the checkbox is not set. Only attach the filter when:
- action === 'adduser'
- the “noconfirmation” checkbox is present
- the correct nonce passes (WordPress core uses action ‘add-user’ with field ‘_wpnonce_add-user’ on the adduser branch, and ‘create-user’ on createuser). (fossies.org)
Minimal fix:
public function handle_adduser_action() { global $pagenow; - // Only proceed if this is not a network admin and noconfirmation is checked - if (current_user_can('manage_network_users') || 'user-new.php' !== $pagenow) { + // Only proceed on the Add Existing User submit from user-new.php. + if (current_user_can('manage_network_users') || 'user-new.php' !== $pagenow) { return; } - // Temporarily grant network user management capability for this request - add_filter('user_has_cap', [$this, 'grant_temp_network_capability'], 10, 3); + $action = isset($_REQUEST['action']) ? sanitize_key(wp_unslash($_REQUEST['action'])) : ''; + $no_confirmation = isset($_POST['noconfirmation']) && '1' === (string) wp_unslash($_POST['noconfirmation']); + + if ('adduser' !== $action || ! $no_confirmation) { + return; + } + + // Verify core nonce used by user-new.php on the adduser branch. + check_admin_referer('add-user', '_wpnonce_add-user'); + + // Temporarily grant manage_network_users for this request. + add_filter('user_has_cap', [$this, 'grant_temp_network_capability'], 10, 3); }inc/class-orphaned-tables-manager.php (1)
179-183: Undefined variable $orphaned_tables; fetch input or recompute.$orphaned_tables is referenced before assignment; PHP emits a notice and the condition always evaluates true, masking logic errors.
One straightforward fix: populate from request if provided, otherwise compute.
- if (empty($orphaned_tables) || ! is_array($orphaned_tables)) { - $orphaned_tables = $this->find_orphaned_tables(); - } + $orphaned_tables = isset($_POST['orphaned_tables']) + ? array_map('sanitize_text_field', (array) wp_unslash($_POST['orphaned_tables'])) + : []; + if (empty($orphaned_tables)) { + $orphaned_tables = $this->find_orphaned_tables(); + }Follow-up: if the Form class already passes the tables in state and validates a nonce, we can tighten this to require the posted list and nonce instead of recomputing.
🧹 Nitpick comments (4)
inc/compat/class-edit-users-compat.php (1)
180-187: Narrow the temporary capability to the adduser+noconfirmation case (defense in depth).Even with the gating in handle_adduser_action, hardening the callback ensures it can’t be abused if attached elsewhere.
Example hardening:
public function grant_temp_network_capability($allcaps, $caps, $args) { - // Only grant during adduser action and if user can create users - if (! empty($allcaps['create_users']) && ! isset($allcaps['manage_network_users'])) { + $action = isset($_REQUEST['action']) ? sanitize_key(wp_unslash($_REQUEST['action'])) : ''; + $no_confirmation = isset($_POST['noconfirmation']) && '1' === (string) wp_unslash($_POST['noconfirmation']); + if ('adduser' === $action && $no_confirmation && ! empty($allcaps['create_users']) && ! isset($allcaps['manage_network_users'])) { $allcaps['manage_network_users'] = true; } return $allcaps; }composer.json (1)
137-140: Excluding “scripts/” may break your Composer post-install step.Your composer scripts call “@php scripts/remove-mpdf-fonts.php” on install/update. If a distribution archive is built with Composer’s archive command honoring this exclude, consumers will install an artifact that can’t run the post-install script (file missing).
Options:
- Keep “scripts/” in archives (remove it from archive.exclude), or
- Move the logic into a PHP class invoked via an autoloaded CLI entrypoint that ships in the archive, or
- Guard the composer script with a file_exists check to no-op when “scripts/remove-mpdf-fonts.php” is absent.
Proposed guard:
"scripts": { - "post-install-cmd": [ - "@php scripts/remove-mpdf-fonts.php" - ], - "post-update-cmd": [ - "@php scripts/remove-mpdf-fonts.php" - ] + "post-install-cmd": [ + "php -r \"file_exists('scripts/remove-mpdf-fonts.php') ? require 'scripts/remove-mpdf-fonts.php' : null;\"" + ], + "post-update-cmd": [ + "php -r \"file_exists('scripts/remove-mpdf-fonts.php') ? require 'scripts/remove-mpdf-fonts.php' : null;\"" + ] },If you’re packaging for wp.org via SVN/GitHub actions instead of composer archive, confirm which packer is authoritative before changing this. I can help wire a build rule if you share your release pipeline.
inc/class-orphaned-tables-manager.php (2)
269-272: DROP TABLE uses %i correctly; safe with the WP 6.2+ guard.Using $wpdb->prepare with the identifier placeholder plus your earlier sanitize_key is appropriate. Consider suppressing errors around individual drops to avoid noisy logs on intermittent schema drift.
Optional tweak:
- $result = $wpdb->query($wpdb->prepare('DROP TABLE IF EXISTS %i', $table)); // phpcs:ignore WordPress.DB.DirectDatabaseQuery, WordPress.DB.PreparedSQLPlaceholders.UnsupportedIdentifierPlaceholder + $wpdb->suppress_errors(true); + $result = $wpdb->query($wpdb->prepare('DROP TABLE IF EXISTS %i', $table)); // phpcs:ignore WordPress.DB.DirectDatabaseQuery, WordPress.DB.PreparedSQLPlaceholders.UnsupportedIdentifierPlaceholder + $wpdb->suppress_errors(false);
221-241: Optional: cut DB work by filtering SHOW TABLES with LIKE.SHOW TABLES returns every table; on large hosts this can be big. You can reduce the set at the server with a LIKE pattern on the network’s prefix + “%_”.
Illustrative change:
- $all_tables = $wpdb->get_col('SHOW TABLES'); // phpcs:ignore WordPress.DB.DirectDatabaseQuery + $like = $wpdb->esc_like($wpdb->prefix) . '%\_%'; + $all_tables = $wpdb->get_col($wpdb->prepare('SHOW TABLES LIKE %s', $like)); // phpcs:ignore WordPress.DB.DirectDatabaseQuery
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
composer.json(1 hunks)inc/checkout/class-checkout.php(1 hunks)inc/class-orphaned-tables-manager.php(2 hunks)inc/compat/class-edit-users-compat.php(1 hunks)lang/multisite-ultimate.pot(24 hunks)
✅ Files skipped from review due to trivial changes (2)
- inc/checkout/class-checkout.php
- lang/multisite-ultimate.pot
⏰ 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: cypress (8.1, chrome)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: PHP 7.4
🔇 Additional comments (2)
composer.json (1)
137-140: Omitting “mu-plugins/” from the Composer archive is reasonable.Given wp.org constraints in this PR, excluding mu-plugins from the distributed plugin is sensible and reduces package size. No action needed.
inc/class-orphaned-tables-manager.php (1)
33-38: Good guard: only register on WP ≥ 6.2.The %i identifier placeholder in $wpdb->prepare is only available in recent WordPress. Early-returning avoids fatal/unsupported placeholder errors on older cores.
| "encrypt-sectrets.php", | ||
| "scripts", | ||
| "mu-plugins" | ||
| ] |
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.
Typo in archive.exclude entry: “encrypt-sectrets.php”.
If the intent was to exclude “encrypt-secrets.php”, the current misspelling will not match and the file will be shipped.
Patch:
- "encrypt-sectrets.php",
+ "encrypt-secrets.php",📝 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.
| "encrypt-sectrets.php", | |
| "scripts", | |
| "mu-plugins" | |
| ] | |
| "encrypt-secrets.php", | |
| "scripts", | |
| "mu-plugins" | |
| ] |
🤖 Prompt for AI Agents
In composer.json around lines 137-140, the archive.exclude entry contains a typo
"encrypt-sectrets.php" which won't match the intended file; update that entry to
"encrypt-secrets.php" so the correct file is excluded from the archive, then
validate the JSON and run any packaging/validation steps (e.g., composer
validate) to ensure the change is applied.
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
🧹 Nitpick comments (7)
assets/css/invoice.css (7)
38-42: Duplicate and inconsistent heading rules (td vs th); consolidate to a single selector.Heading styles are spread across three blocks, mix
tdandth, and partially overlap. This invites maintenance drift and unpredictable overrides. Consolidate to a single rule that targets boththandtd. Also set a darker text color to ensure adequate contrast over#eee.Apply this consolidation:
-.invoice-box table tr.heading td { - background: #eee; - border-bottom: 1px solid #ddd; - font-weight: 500; -} - -.invoice-box table tr.heading th { - border-left: 1px solid #ddd; - border-right: 1px solid #ddd; -} - -.invoice-box table tr.heading th { - background: #eee; - border-top: 1px solid #ddd; - border-bottom: 1px solid #ddd; - padding: 10px; - text-align: right; - font-weight: bold; - text-transform: uppercase; - font-size: 80%; -} +/* Consolidated heading styles for both th and td */ +.invoice-box table tr.heading > th, +.invoice-box table tr.heading > td { + background: #eee; + border-top: 1px solid #ddd; + border-right: 1px solid #ddd; + border-bottom: 1px solid #ddd; + border-left: 1px solid #ddd; + padding: 10px; + text-align: right; + font-weight: 700; + text-transform: uppercase; + font-size: 80%; + color: #333; /* better contrast on #eee */ +}Also applies to: 48-51, 57-66
20-22: Make numeric-column alignment resilient and RTL-friendly without hard-coding column index.Relying on
td:nth-child(2)assumes the amount is always in column 2. If columns change, alignment breaks and requires the RTL override. Prefer aligning the last column and use logical alignment (start/end) to avoid separate RTL rules.Apply this refactor and verify that the amounts indeed sit in the last column in the template:
-.invoice-box table tr td:nth-child(2) { - text-align: right; -} +/* Align totals/amounts in the last column */ +.invoice-box table tr > *:last-child { + text-align: end; /* resolves automatically for RTL */ +} @@ -.rtl table tr td:nth-child(2) { - text-align: left; -} +/* No override needed when using logical alignment */If some rows have action buttons or notes on the far right, consider adding a semantic class (e.g.,
.col-amount) on the amount cell and target that instead:
.invoice-box .col-amount { text-align: end; }Also applies to: 113-115
81-87: Totals row: avoid forcing all cells to right-align.Right-aligning every cell can hurt readability when labels should stay left/start-aligned. Right-align only the amount cell (e.g.,
:last-child) to keep labels readable.Apply this tweak if labels appear misaligned:
-.invoice-box table tr.total td { +.invoice-box table tr.total td:last-child { border-top: 2px solid #eee; font-weight: bold; padding-bottom: 60px; padding-top: 10px; - text-align: right; + text-align: end; }
89-101: Responsive rules look good; consider also reducing huge title size on narrow screens.The stacking works nicely. On small screens the 45px title can overflow. Consider scaling it down under 600px.
Suggested addition:
@media only screen and (max-width: 600px) { .invoice-box table tr.top table td { width: 100%; display: block; text-align: center; } @@ .invoice-box table tr.information table td { width: 100%; display: block; text-align: center; } + .invoice-box table tr.top table td.title { + font-size: 32px; + line-height: 36px; + } }
103-111: RTL base is solid; prefer logical properties to reduce overrides.Since this layout is table-based, switching to logical text alignment (
start/end) can remove the need for extra RTL overrides and reduce maintenance surface.As in earlier comments, favor
text-align: start|endand avoid left/right-specific rules where possible. No action required if you need to support very old browsers.
1-7: Optional: set a practical max-width for nicer on-screen reading.
width: 100%is fine for PDF/print, but amax-width(e.g., 800px) improves readability in the browser preview while still printing full-width as needed.Example:
.invoice-box { - width: 100%; + width: 100%; + max-width: 800px; margin: auto; font-size: 16px; line-height: 24px; color: #555; }
115-115: Add minimal print rules to improve PDF/print fidelity.Backgrounds are often dropped by printers unless explicitly allowed. These hints improve consistency across HTML-to-PDF engines and native print.
Apply at end of file:
+@media print { + /* Try to preserve backgrounds and colors in print/PDF where supported */ + * { + -webkit-print-color-adjust: exact; + print-color-adjust: exact; + } + /* Avoid row splits where possible */ + .invoice-box table tr { + break-inside: avoid; + page-break-inside: avoid; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
assets/css/invoice.css(1 hunks)assets/css/invoice.min.css(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- assets/css/invoice.min.css
⏰ 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). (13)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.0, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (7.4, firefox)
🔇 Additional comments (1)
assets/css/invoice.css (1)
1-7: Good move: stylesheet extraction and scoped base styles.Shifting invoice styling out of inline CSS into a dedicated, scoped stylesheet is a solid step for wp.org compliance and maintainability. The base
.invoice-boxrules are concise and well-scoped.
| .invoice-box table { | ||
| border-collapse: 1px; | ||
| } |
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.
Invalid value for border-collapse; browsers will ignore this.
border-collapse: 1px; is not valid CSS. Use collapse (and add border-spacing: 0; for completeness). Otherwise you’ll get separated borders and inconsistent rendering across HTML-to-PDF engines.
Apply this diff:
.invoice-box table {
- border-collapse: 1px;
+ border-collapse: collapse;
+ border-spacing: 0;
}📝 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.
| .invoice-box table { | |
| border-collapse: 1px; | |
| } | |
| .invoice-box table { | |
| border-collapse: collapse; | |
| border-spacing: 0; | |
| } |
🤖 Prompt for AI Agents
In assets/css/invoice.css around lines 44 to 46, the rule uses an invalid value
"border-collapse: 1px;" which browsers ignore; change it to "border-collapse:
collapse" and add "border-spacing: 0" to remove gaps so table borders render
consistently (especially for HTML-to-PDF engines). Ensure you update that
selector to use the correct property values and keep formatting consistent with
the file.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
views/base/addons/details.php (4)
85-88: External link opened in a new tab lacks rel="noopener"; also use esc_url for href.Prevents reverse tabnabbing and aligns with WP escaping best practices.
Apply:
- <a class="wu-no-underline" target="_blank" href="<?php echo esc_attr($addon->permalink); ?>"> + <a class="wu-no-underline" target="_blank" rel="noopener noreferrer" href="<?php echo esc_url( $addon->permalink ); ?>">
109-117: Logic bug: “Premium” label condition inverted due to operator precedence.
! $price > 0evaluates as(! $price) > 0. Shows “Premium” for free add-ons.Apply:
- <?php if (! $addon->prices['price'] > 0) : ?> + <?php if ( isset( $addon->prices['price'] ) && (float) $addon->prices['price'] > 0 ) : ?> <span class="wu-text-green-800 wu-inline-block wu-py-1"> - <?php esc_html_e('This is a Premium Add-on.', 'multisite-ultimate'); ?> + <?php esc_html_e( 'This is a Premium Add-on.', 'multisite-ultimate' ); ?> </span> <?php endif; ?>
131-154: Free install branch misses price == 0; possible notice on undefined download_url key.Use <= 0 for free, and guard download_url with !empty().
Apply:
- <?php if ($addon->extensions['wp-update-server-plugin']['download_url'] || (float) $addon->prices['price'] < 0) : ?> + <?php if ( ! empty( $addon->extensions['wp-update-server-plugin']['download_url'] ) || (float) $addon->prices['price'] <= 0 ) : ?>
147-151: Build purchase URL safely and escape correctly.Use add_query_arg with esc_url and cast id to int.
Apply:
- href="<?php echo esc_attr($addon->permalink . '?add-to-cart=' . $addon->id); ?>" + href="<?php echo esc_url( add_query_arg( 'add-to-cart', (int) $addon->id, $addon->permalink ) ); ?>"readme.txt (1)
7-11: Header fixes for WP.org readme: PHP version granularity and GPL link.
- “Requires PHP” should be a major.minor (e.g., 7.4), not a patch-level (7.4.30).
- Prefer “GPLv2 or later” and HTTPS license URI.
-Requires PHP: 7.4.30 +Requires PHP: 7.4 ... -Stable tag: 2.4.3 -License: GPLv2 -License URI: http://www.gnu.org/licenses/gpl-2.0.html +Stable tag: 2.4.3 +License: GPLv2 or later +License URI: https://www.gnu.org/licenses/gpl-2.0.html
🧹 Nitpick comments (5)
views/base/addons/details.php (1)
14-14: Inline style is fine, but minor nit: quote the URL in CSS.Quoting handles spaces/special chars more robustly.
Apply:
-<div id="plugin-information" style="position: static"> +<div id="plugin-information" style="position: static">And later:
-style="background-image:url(<?php echo esc_url($addon->images[0]['thumbnail']); ?>);background-position:center;" +style="background-image:url('<?php echo esc_url( $addon->images[0]['thumbnail'] ); ?>');background-position:center;"readme.txt (4)
220-235: Clarify credential handling and transport guarantees for hosting APIs.Add a sentence about storage, scoping, and deletion of API credentials; explicitly state all requests use HTTPS and configurable timeouts.
**Closte Hosting API** - Service: Integration with Closte hosting provider for automated site management -- Data sent: Site configuration data, API credentials +- Data sent: Site configuration data; API credentials are stored in your WordPress database (admin-only access) and can be revoked at any time. +- Transport: All requests use HTTPS with configurable timeouts/retries. ... **Cloudways Hosting API** - Service: Integration with Cloudways hosting provider for automated site management -- Data sent: Site configuration data, API credentials, authentication tokens +- Data sent: Site configuration data; API credentials/tokens are stored in your WordPress database (admin-only access) and can be revoked at any time. +- Transport: All requests use HTTPS with configurable timeouts/retries.
250-259: Minor privacy clarification for DNS lookups.DNS queries reveal the queried hostname to Google DNS. Call this out and note that no user identifiers are sent.
**Google DNS Resolution** - Service: DNS lookup service for domain verification -- Data sent: Domain names for DNS lookup (no personal data) +- Data sent: The queried domain/record name only (no user identifiers).
268-269: Newsletter: make opt-in and unsubscribe explicit.Strengthen language to align with WP.org user-consent expectations.
- Terms of Service: https://multisiteultimate.com/terms-of-service/ - Privacy Policy: https://multisiteultimate.com/privacy-policy/ + - Consent: Opt‑in only during setup; every email includes an unsubscribe link.
291-299: Changelog vs stable tag: avoid placeholder dates or bump stable tag when releasing.Either mark 2.4.4 as “Unreleased” or set Stable tag to 2.4.4 with a real date on release.
-Version [2.4.4] - Released on 2025-08-XX +Version [2.4.4] - UnreleasedWhen you ship 2.4.4 to wp.org: update “Stable tag” to 2.4.4 and replace “Unreleased” with the actual date.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
assets/js/support.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (4)
assets/js/support.js(0 hunks)readme.txt(4 hunks)views/base/addons.php(1 hunks)views/base/addons/details.php(2 hunks)
💤 Files with no reviewable changes (1)
- assets/js/support.js
🧰 Additional context used
🪛 LanguageTool
readme.txt
[grammar] ~166-~166: There might be a mistake here.
Context: ... Services = MaxMind GeoLite2 Database - Service: Geolocation database for determ...
(QB_NEW_EN)
[grammar] ~167-~167: There might be a mistake here.
Context: ...mining user location based on IP address - Data sent: No personal data is sent - on...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...ly when geolocation features are enabled - Service URL: http://geolite.maxmind.com/...
(QB_NEW_EN)
[grammar] ~170-~170: There might be a mistake here.
Context: ...d/geoip/database/GeoLite2-Country.tar.gz - Terms of Service: https://www.maxmind.co...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...ps://www.maxmind.com/en/terms-of-service - Privacy Policy: https://www.maxmind.com/...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: .../en/privacy-policy IP Lookup Services - Service: External APIs to determine the ...
(QB_NEW_EN)
[grammar] ~175-~175: There might be a mistake here.
Context: ...o determine the user's public IP address - Data sent: HTTP request to determine IP ...
(QB_NEW_EN)
[grammar] ~176-~176: There might be a mistake here.
Context: ...ine IP address (no personal data stored) - When: When geolocation features are enab...
(QB_NEW_EN)
[grammar] ~177-~177: There might be a mistake here.
Context: ...abled and user IP needs to be determined - Services used: - IPify API (http://api...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ... needs to be determined - Services used: - IPify API (http://api.ipify.org/) - Term...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...fy.org/) - Terms: https://www.ipify.org/ - IP Echo (http://ipecho.net/plain) - Id...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...g/ - IP Echo (http://ipecho.net/plain) - Ident.me (http://ident.me) - WhatIsMyI...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...et/plain) - Ident.me (http://ident.me) - WhatIsMyIPAddress (http://bot.whatismyip...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...dress (http://bot.whatismyipaddress.com) - IPinfo.io (https://ipinfo.io/) - Terms: ...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...fo.io/) - Terms: https://ipinfo.io/terms - IP-API.com (http://ip-api.com/) - Terms:...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: ...ns = Multisite Ultimate Update Server - Service: Official update server for the ...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ... and its add-ons (multisiteultimate.com) - Data sent: Site URL, plugin version, lic...
(QB_NEW_EN)
[grammar] ~190-~190: There might be a mistake here.
Context: ...ion, license keys, authentication tokens - When: During plugin/add-on updates and l...
(QB_NEW_EN)
[grammar] ~191-~191: There might be a mistake here.
Context: ...plugin/add-on updates and license checks - Terms of Service: https://multisiteultim...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: .../multisiteultimate.com/terms-of-service/ - Privacy Policy: https://multisiteultimat...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...com/privacypolicy/ Closte Hosting API - Service: Integration with Closte hosting...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...g provider for automated site management - Data sent: Site configuration data, API ...
(QB_NEW_EN)
[grammar] ~222-~222: There might be a mistake here.
Context: ...Site configuration data, API credentials - When: Only when Closte integration is en...
(QB_NEW_EN)
[grammar] ~223-~223: There might be a mistake here.
Context: ...te integration is enabled and configured - Service URL: https://app.closte.com/api/...
(QB_NEW_EN)
[grammar] ~224-~224: There might be a mistake here.
Context: ...e URL: https://app.closte.com/api/client - Terms of Service: https://closte.com/ter...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...ce: https://closte.com/terms-of-service/ - Privacy Policy: https://closte.com/priva...
(QB_NEW_EN)
[grammar] ~228-~228: There might be a mistake here.
Context: ...privacy-policy/ Cloudways Hosting API - Service: Integration with Cloudways host...
(QB_NEW_EN)
[grammar] ~229-~229: There might be a mistake here.
Context: ...g provider for automated site management - Data sent: Site configuration data, API ...
(QB_NEW_EN)
[grammar] ~230-~230: There might be a mistake here.
Context: ..., API credentials, authentication tokens - When: Only when Cloudways integration is...
(QB_NEW_EN)
[grammar] ~231-~231: There might be a mistake here.
Context: ...ys integration is enabled and configured - Service URL: https://api.cloudways.com/a...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ....cloudways.com/api/v1/oauth/access_token - Terms of Service: https://www.cloudways....
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...: https://www.cloudways.com/en/terms.php - Privacy Policy: https://www.cloudways.co...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...main Services = Google DNS Resolution - Service: DNS lookup service for domain v...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...S lookup service for domain verification - Data sent: Domain names for DNS lookup (...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ... names for DNS lookup (no personal data) - When: During domain mapping setup and ve...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...ng domain mapping setup and verification - Service URL: https://dns.google/resolve ...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ... Service URL: https://dns.google/resolve - Terms of Service: https://developers.goo...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ce: https://developers.google.com/terms/ - Privacy Policy: https://policies.google....
(QB_NEW_EN)
[grammar] ~268-~268: There might be a mistake here.
Context: .../multisiteultimate.com/terms-of-service/ - Privacy Policy: https://multisiteultimat...
(QB_NEW_EN)
⏰ 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). (13)
- GitHub Check: PHP 8.0
- GitHub Check: PHP 8.3
- GitHub Check: PHP 8.1
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (7.4, firefox)
🔇 Additional comments (3)
views/base/addons.php (1)
8-10: Good hardening: ABSPATH guard added.Standard and correct placement before output. No further action.
views/base/addons/details.php (1)
8-10: Good hardening: ABSPATH guard added.Standard and correct placement before output. No further action.
readme.txt (1)
2-2: Confirm “Contributors” are WordPress.org usernames.WP.org expects plugin directory usernames, not GitHub handles.
Would you like a quick grep across the repo to sync the contributor list in README.md and readme.txt?
| = Geolocation Services = | ||
|
|
||
| **MaxMind GeoLite2 Database** | ||
| - Service: Geolocation database for determining user location based on IP address | ||
| - Data sent: No personal data is sent - only downloads the database file | ||
| - When: Downloaded periodically when geolocation features are enabled | ||
| - Service URL: http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.tar.gz | ||
| - Terms of Service: https://www.maxmind.com/en/terms-of-service | ||
| - Privacy Policy: https://www.maxmind.com/en/privacy-policy | ||
|
|
||
| **IP Lookup Services** | ||
| - Service: External APIs to determine the user's public IP address | ||
| - Data sent: HTTP request to determine IP address (no personal data stored) | ||
| - When: When geolocation features are enabled and user IP needs to be determined | ||
| - Services used: | ||
| - IPify API (http://api.ipify.org/) - Terms: https://www.ipify.org/ | ||
| - IP Echo (http://ipecho.net/plain) | ||
| - Ident.me (http://ident.me) | ||
| - WhatIsMyIPAddress (http://bot.whatismyipaddress.com) | ||
| - IPinfo.io (https://ipinfo.io/) - Terms: https://ipinfo.io/terms | ||
| - IP-API.com (http://ip-api.com/) - Terms: https://ip-api.com/docs/legal | ||
|
|
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.
🛠️ Refactor suggestion
Use HTTPS everywhere, fix MaxMind download info, and correct GDPR wording on IP-as-PII.
- Replace all http:// endpoints with HTTPS. WP.org guidelines expect TLS for external calls where available.
- MaxMind GeoLite2 requires a licensed download URL since Dec 2019; the legacy geolite.maxmind.com URL is deprecated. Use the license-key URL and document that an account + license acceptance are required.
- IP addresses are personal data under GDPR. “No personal data” is inaccurate for IP lookup. Clarify that the user’s public IP is disclosed to the chosen service and make it optional/opt-in.
- ip-api.com’s free tier is HTTP-only. Either remove it or clearly state it’s only used when a paid HTTPS plan is configured.
Apply this doc patch:
-**MaxMind GeoLite2 Database**
- - Service: Geolocation database for determining user location based on IP address
- - Data sent: No personal data is sent - only downloads the database file
- - When: Downloaded periodically when geolocation features are enabled
- - Service URL: http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.tar.gz
- - Terms of Service: https://www.maxmind.com/en/terms-of-service
- - Privacy Policy: https://www.maxmind.com/en/privacy-policy
+**MaxMind GeoLite2 Database**
+ - Service: Geolocation database used locally to map IP → country/region.
+ - Data sent: None (database is downloaded to your server).
+ - When: Downloaded periodically when geolocation features are enabled.
+ - Service URL: https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-Country&license_key=YOUR_KEY&suffix=tar.gz
+ - Note: Requires a free MaxMind account and accepting the GeoLite2 EULA.
+ - Terms of Service: https://www.maxmind.com/en/terms-of-service
+ - Privacy Policy: https://www.maxmind.com/en/privacy-policy
-**IP Lookup Services**
- - Service: External APIs to determine the user's public IP address
- - Data sent: HTTP request to determine IP address (no personal data stored)
- - When: When geolocation features are enabled and user IP needs to be determined
+**IP Lookup Services**
+ - Service: External APIs to determine the user's public IP address.
+ - Data sent: The user's public IP address is disclosed to the selected service. Considered personal data under GDPR. Not stored by the plugin.
+ - When: Only if geolocation features are enabled and IP detection is required (opt-in).
- Services used:
- - IPify API (http://api.ipify.org/) - Terms: https://www.ipify.org/
- - IP Echo (http://ipecho.net/plain)
- - Ident.me (http://ident.me)
- - WhatIsMyIPAddress (http://bot.whatismyipaddress.com)
- - IPinfo.io (https://ipinfo.io/) - Terms: https://ipinfo.io/terms
- - IP-API.com (http://ip-api.com/) - Terms: https://ip-api.com/docs/legal
+ - IPify API (https://api.ipify.org) — Terms: https://www.ipify.org/
+ - Ident.me (https://ident.me)
+ - WhatIsMyIP (suggest HTTPS alternative) e.g., remove or replace with a TLS-capable endpoint.
+ - IPinfo.io (https://ipinfo.io) — Terms: https://ipinfo.io/terms
+ - IP-API.com (HTTPS available for paid/pro plans only) — Terms: https://ip-api.com/docs/legal
+ - IP Echo (https://ipecho.net/plain) if available; otherwise consider removing due to HTTP-only.📝 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.
| = Geolocation Services = | |
| **MaxMind GeoLite2 Database** | |
| - Service: Geolocation database for determining user location based on IP address | |
| - Data sent: No personal data is sent - only downloads the database file | |
| - When: Downloaded periodically when geolocation features are enabled | |
| - Service URL: http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.tar.gz | |
| - Terms of Service: https://www.maxmind.com/en/terms-of-service | |
| - Privacy Policy: https://www.maxmind.com/en/privacy-policy | |
| **IP Lookup Services** | |
| - Service: External APIs to determine the user's public IP address | |
| - Data sent: HTTP request to determine IP address (no personal data stored) | |
| - When: When geolocation features are enabled and user IP needs to be determined | |
| - Services used: | |
| - IPify API (http://api.ipify.org/) - Terms: https://www.ipify.org/ | |
| - IP Echo (http://ipecho.net/plain) | |
| - Ident.me (http://ident.me) | |
| - WhatIsMyIPAddress (http://bot.whatismyipaddress.com) | |
| - IPinfo.io (https://ipinfo.io/) - Terms: https://ipinfo.io/terms | |
| - IP-API.com (http://ip-api.com/) - Terms: https://ip-api.com/docs/legal | |
| = Geolocation Services = | |
| **MaxMind GeoLite2 Database** | |
| - Service: Geolocation database used locally to map IP → country/region. | |
| - Data sent: None (database is downloaded to your server). | |
| - When: Downloaded periodically when geolocation features are enabled. | |
| - Service URL: https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-Country&license_key=YOUR_KEY&suffix=tar.gz | |
| - Note: Requires a free MaxMind account and accepting the GeoLite2 EULA. | |
| - Terms of Service: https://www.maxmind.com/en/terms-of-service | |
| - Privacy Policy: https://www.maxmind.com/en/privacy-policy | |
| **IP Lookup Services** | |
| - Service: External APIs to determine the user's public IP address. | |
| - Data sent: The user's public IP address is disclosed to the selected service. Considered personal data under GDPR. Not stored by the plugin. | |
| - When: Only if geolocation features are enabled and IP detection is required (opt-in). | |
| - Services used: | |
| - IPify API (https://api.ipify.org) — Terms: https://www.ipify.org/ | |
| - Ident.me (https://ident.me) | |
| - WhatIsMyIP (replace with a TLS-capable endpoint or remove) | |
| - IPinfo.io (https://ipinfo.io) — Terms: https://ipinfo.io/terms | |
| - IP-API.com (HTTPS available for paid/pro plans only) — Terms: https://ip-api.com/docs/legal | |
| - IP Echo (https://ipecho.net/plain) if available; otherwise consider removing due to HTTP-only. |
🧰 Tools
🪛 LanguageTool
[grammar] ~166-~166: There might be a mistake here.
Context: ... Services = MaxMind GeoLite2 Database - Service: Geolocation database for determ...
(QB_NEW_EN)
[grammar] ~167-~167: There might be a mistake here.
Context: ...mining user location based on IP address - Data sent: No personal data is sent - on...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...ly when geolocation features are enabled - Service URL: http://geolite.maxmind.com/...
(QB_NEW_EN)
[grammar] ~170-~170: There might be a mistake here.
Context: ...d/geoip/database/GeoLite2-Country.tar.gz - Terms of Service: https://www.maxmind.co...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...ps://www.maxmind.com/en/terms-of-service - Privacy Policy: https://www.maxmind.com/...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: .../en/privacy-policy IP Lookup Services - Service: External APIs to determine the ...
(QB_NEW_EN)
[grammar] ~175-~175: There might be a mistake here.
Context: ...o determine the user's public IP address - Data sent: HTTP request to determine IP ...
(QB_NEW_EN)
[grammar] ~176-~176: There might be a mistake here.
Context: ...ine IP address (no personal data stored) - When: When geolocation features are enab...
(QB_NEW_EN)
[grammar] ~177-~177: There might be a mistake here.
Context: ...abled and user IP needs to be determined - Services used: - IPify API (http://api...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ... needs to be determined - Services used: - IPify API (http://api.ipify.org/) - Term...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...fy.org/) - Terms: https://www.ipify.org/ - IP Echo (http://ipecho.net/plain) - Id...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...g/ - IP Echo (http://ipecho.net/plain) - Ident.me (http://ident.me) - WhatIsMyI...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...et/plain) - Ident.me (http://ident.me) - WhatIsMyIPAddress (http://bot.whatismyip...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...dress (http://bot.whatismyipaddress.com) - IPinfo.io (https://ipinfo.io/) - Terms: ...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...fo.io/) - Terms: https://ipinfo.io/terms - IP-API.com (http://ip-api.com/) - Terms:...
(QB_NEW_EN)
🤖 Prompt for AI Agents
readme.txt lines 164-185: update Geolocation Services section to use HTTPS for
all endpoints, replace the deprecated MaxMind geolite.maxmind.com URL with the
licensed GeoLite2 download pattern (document that a MaxMind account and license
acceptance + license key are required for downloads), correct the GDPR wording
to state that public IPs are personal data and that contacting external IP
lookup services will disclose the user's IP (make these lookups explicit
opt-in/optional), and address ip-api.com by removing it from default list or
adding a clear note that the free tier is HTTP-only and will only be used if a
paid HTTPS plan is configured; ensure Service URL, Terms and Privacy links use
HTTPS where available and that the text indicates account/license requirements
for MaxMind and the opt-in nature of IP lookups.
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: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (36)
inc/sso/class-sso.php (2)
485-491: Harden redirect against open-redirects.return_url comes from input; prefer wp_safe_redirect and validate/sanitize the URL.
- $return_url = remove_query_arg('sso_verify', sanitize_text_field(wp_unslash($_GET['return_url'] ?? ''))); // phpcs:ignore WordPress.Security.NonceVerification + $return_url = remove_query_arg( + 'sso_verify', + wp_sanitize_redirect( (string) wp_unslash( $_GET['return_url'] ?? '' ) ) // phpcs:ignore WordPress.Security.NonceVerification + ); @@ - wp_redirect($url, 303, 'WP-Ultimo-SSO'); + wp_safe_redirect($url, 303, 'WP-Ultimo-SSO');
327-334: Avoid potential fatal when broker is unavailable.Empty guard leads to
$broker->is_must_redirect_call()on null.- if ( ! $broker) { - } + if ( ! $broker ) { + return null; + }inc/managers/class-gateway-manager.php (1)
320-327: Do not surface raw exception messages to users; log details and show a generic errorDirectly exposing $e->getMessage() may leak internal paths or sensitive info. Log the exception and display a generic, translated string with a back link.
- wp_die( - esc_html($e->getMessage()), - esc_html__('Error', 'multisite-ultimate'), - [ - 'back_link' => true, - 'response' => '200', - ] - ); + wu_log_add( + 'wu-gateway-confirm-errors', + sprintf( + 'Gateway confirmation exception: %s in %s:%d%s%s', + $e->getMessage(), + $e->getFile(), + $e->getLine(), + PHP_EOL, + $e->getTraceAsString() + ), + LogLevel::ERROR + ); + wp_die( + esc_html__('Something went wrong while confirming your payment. Please try again or contact support.', 'multisite-ultimate'), + esc_html__('Error', 'multisite-ultimate'), + [ + 'back_link' => true, + 'response' => '200', + ] + );views/checkout/templates/pricing-table/legacy.php (3)
32-42: Fix boolean reduction: legacy mode currently reflects only the last recurring productAccumulator is overwritten per-iteration, so the final value equals the last comparison, not “all have same duration.” Use an AND accumulation with an explicit true initial value; also short-circuit when no recurring product exists.
-$legacy_mode = array_reduce( - $products_to_reduce, - function ($all_have_same_duration, $product) use ($first_recurring_product) { +$legacy_mode = ( null !== $first_recurring_product ) && array_reduce( + $products, + function ( $all_have_same_duration, $product ) use ( $first_recurring_product ) { if ($product && $product->is_recurring()) { - $all_have_same_duration = $first_recurring_product->get_recurring_description() === $product->get_recurring_description(); + return $all_have_same_duration && ( $first_recurring_product->get_recurring_description() === $product->get_recurring_description() ); } return $all_have_same_duration; - } -); + }, + true +);
44-61: Undefined variable: $force_different_durations can trigger noticesThis variable is used but never defined in this template. Default it to false to avoid PHP notices (wp.org requirement).
+// Ensure default to avoid notices if not provided by caller. +if ( ! isset( $force_different_durations ) ) { + $force_different_durations = false; +} wp_add_inline_script( 'wu-checkout',
206-210: Currency symbol not printed (esc_html() return value is discarded)esc_html() returns a string; it doesn’t echo. The symbol is currently missing in the UI.
- <?php esc_html(wu_get_currency_symbol($product->get_currency())); ?> + <?php echo esc_html( wu_get_currency_symbol( $product->get_currency() ) ); ?>inc/list-tables/class-inside-events-list-table.php (1)
81-102: Escape event message in title to avoid XSS
$item->get_message()is unescaped user/content data. Wrap it to prevent injection when rendered by the template.Apply this diff:
- 'title' => sprintf('<span class="wu-font-normal">%s</span>', wp_trim_words($item->get_message(), 15)), + 'title' => sprintf( + '<span class="wu-font-normal">%s</span>', + esc_html( + wp_trim_words( + wp_strip_all_tags((string) $item->get_message()), + 15 + ) + ) + ),Additionally, ensure the manual initiator display name is escaped when building
$value:$display_name = esc_html($item->get_author_display_name()); $value = sprintf('<span class="wu-text-gray-600">%s%s</span>', $avatar, $display_name);views/dashboard-statistics/widget-countries.php (1)
86-97: Incorrect “Other” count: $_state_count never increments.As written, “Other” equals the total country count, which is wrong. Increment the accumulator inside the loop.
Apply this diff:
<?php foreach ($state_list as $state => $state_count) : ?> <tr> <td class="wu-text-xs">|⟶ <?php echo esc_html($state); ?></td> <td class="wu-text-right"><?php echo esc_html($state_count); ?></td> </tr> + <?php $_state_count += (int) $state_count; ?> <?php endforeach; ?>inc/ui/class-site-maintenance-element.php (1)
269-303: Add a defensive guard if $this->site isn’t set.Prevents accidental fatal calls when setup() isn’t run; mirrors pattern used in other elements.
public function output($atts, $content = null): void { + if ( ! $this->site ) { + _doing_it_wrong(__METHOD__, esc_html__('setup() or setup_preview() must be called before output().', 'multisite-ultimate')); + return; + }inc/admin-pages/class-customer-edit-admin-page.php (1)
949-956: Escape $country_name to prevent XSS.$country_name is output unescaped in both branches. Escape for HTML/attribute contexts.
- '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>', - $country_name, + '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>', + esc_html($country_name), esc_attr($country_name), wu_get_flag_emoji((string) $country_code) ); } else { - $html = $country_name; + $html = esc_html($country_name); }inc/ui/class-template-switching-element.php (2)
317-326: Guard output if $this->site is null.Aligns with other elements and prevents accidental usage without setup().
public function output($atts, $content = null): void { + if ( ! $this->site ) { + _doing_it_wrong(__METHOD__, esc_html__('setup() or setup_preview() must be called before output().', 'multisite-ultimate')); + return; + }
268-304: AJAX: handle failure path and sanitize template_id.Currently no response on failure; also cast template_id to int to avoid invalid input.
- $template_id = wu_request('template_id', ''); + $template_id = (int) wu_request('template_id', 0); if ( ! $template_id) { return new \WP_Error('template_id_required', __('You need to provide a valid template to duplicate.', 'multisite-ultimate')); } $switch = \WP_Ultimo\Helpers\Site_Duplicator::override_site($template_id, $this->site->get_id()); @@ - if ($switch) { + if ($switch) { wp_send_json_success( [ 'redirect_url' => add_query_arg( [ 'updated' => 1, ], $referer ), ] ); + } else { + wp_send_json_error(new \WP_Error('switch_failed', __('Template switch failed. Please try again.', 'multisite-ultimate'))); }inc/ui/class-login-form-element.php (3)
510-517: Fix main_site redirect branch; currently exits without redirect.This causes a white screen post-login. Redirect to the network main site with the requested path.
- } elseif ('main_site' === $redirect_type) { - exit; + } elseif ('main_site' === $redirect_type) { + wp_safe_redirect(network_site_url($requested_redirect_to ?: '/')); + exit; }
606-610: Incorrect rp_key comparison; always true and skips validation.Compare the POST rp_key against the cookie-derived $rp_key.
- if (isset($_POST['pass1']) && isset($_POST['rp_key']) && ! hash_equals(wp_unslash($_POST['rp_key']), wp_unslash($_POST['rp_key']))) { // phpcs:ignore WordPress.Security.NonceVerification + if (isset($_POST['pass1']) && isset($_POST['rp_key']) && ! hash_equals((string) $rp_key, (string) wp_unslash($_POST['rp_key']))) { // phpcs:ignore WordPress.Security.NonceVerification $user = false; }
586-591: Escape URL in confirmation link.Use esc_url() inside the sprintf.
- __('Check your email for the confirmation link, then visit the <a href="%s">login page</a>.'), // phpcs:ignore WordPress.WP.I18n.MissingArgDomain - wp_login_url() + __('Check your email for the confirmation link, then visit the <a href="%s">login page</a>.'), // phpcs:ignore WordPress.WP.I18n.MissingArgDomain + esc_url(wp_login_url())inc/ui/class-billing-info-element.php (3)
259-271: Update the output() docblock to reflect void output and fix unused param warning.The docblock still says “Should return HTML” and
@return string, but the method now prints and returns void. Also,$contentis unused (PHPMD warning).Apply this diff:
- * Should return HTML markup to be used to display the block. + * Prints the HTML markup for this block. @@ - * @param array $atts Parameters of the block/shortcode. - * @param string|null $content The content inside the shortcode. - * @return string + * @param array $atts Parameters of the block/shortcode. + * @param string|null $content Unused. Kept for signature compatibility with renderers. + * @return void + * @SuppressWarnings(PHPMD.UnusedFormalParameter)
279-291: Merge element defaults/atts before passing to the template.Other elements (e.g., Account_Summary_Element) merge
$this->atts; here it’s missing, so template-level defaults (liketitle) may be skipped.Apply this diff:
public function output($atts, $content = null): void { @@ - $atts['membership'] = $this->membership; + // Keep parity with other elements: merge saved/derived atts first. + $atts = array_merge($atts, $this->atts); + $atts['membership'] = $this->membership;
377-411: Add nonce and ownership/capability checks to prevent unauthorized updates.The handler trusts a membership hash from the request and only requires
existcapability. Any logged-in user who learns another membership hash could change its billing address.Apply this diff (adapt method names to your model as needed):
public function handle_update_billing_address(): void { + // CSRF protection. + if (function_exists('check_ajax_referer')) { + check_ajax_referer('update_billing_address'); + } @@ - $membership = wu_get_membership_by_hash(wu_request('membership')); + $membership = wu_get_membership_by_hash(wu_request('membership')); if ( ! $membership) { $error = new \WP_Error('membership-dont-exist', __('Something went wrong.', 'multisite-ultimate')); wp_send_json_error($error); } + + // Authorization: ensure the current user can edit this membership. + $current_user_id = get_current_user_id(); + $owner_id = method_exists($membership, 'get_customer_id') + ? (int) $membership->get_customer_id() + : (method_exists($membership, 'get_user_id') ? (int) $membership->get_user_id() : 0); + + if ( ! current_user_can('manage_network') && $owner_id !== $current_user_id) { + wp_send_json_error(new \WP_Error('wu-forbidden', __('You are not allowed to update this membership.', 'multisite-ultimate')), 403); + }If your form system already injects/verifies nonces centrally, reply and I’ll tailor this to hook into it instead.
inc/functions/markup-helpers.php (1)
157-164: Escape URL and label in wu_preview_image() to prevent XSS.
$image_urland$labelare inserted unescaped.Apply this diff:
function wu_preview_image($image_url, $label = false): string { if (empty($label)) { $label = __('Preview', 'multisite-ultimate'); } - return sprintf(' <span class="wu-image-preview wu-text-gray-600 wu-bg-gray-200 wu-p-1 wu-px-2 wu-ml-1 wu-inline-block wu-text-2xs wu-uppercase wu-font-bold wu-rounded wu-cursor-pointer wu-border-gray-300 wu-border wu-border-solid" data-image="%s">%s %s</span>', $image_url, "<span class='dashicons-wu-image wu-align-middle wu-mr-1'></span>", $label); + return sprintf( + ' <span class="wu-image-preview wu-text-gray-600 wu-bg-gray-200 wu-p-1 wu-px-2 wu-ml-1 wu-inline-block wu-text-2xs wu-uppercase wu-font-bold wu-rounded wu-cursor-pointer wu-border-gray-300 wu-border wu-border-solid" data-image="%s">%s %s</span>', + esc_url($image_url), + "<span class='dashicons-wu-image wu-align-middle wu-mr-1' aria-hidden='true'></span>", + esc_html($label) + ); }inc/admin-pages/class-broadcast-edit-admin-page.php (1)
188-239: Replace switch-with-conditions; fix avatar/name placeholder orderThe current switch compares an array to boolean conditions, which is fragile and confusing. Also, in the single-target case, the avatar and display name placeholders are swapped, placing the avatar inside . Use a clear if/elseif and map placeholders correctly.
Apply:
- switch ($targets) { - case $targets_count < 0: + if ($targets_count <= 0) { printf( "<span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> <div class=''> <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> </div>", esc_html__('No customer found', 'multisite-ultimate') ); - - break; - case 1 === $targets_count: + } elseif (1 === $targets_count) { $customer = wu_get_customer($targets[0]); $url_atts = [ 'id' => $customer->get_id(), ]; $customer_link = wu_network_admin_url('wp-ultimo-edit-customer', $url_atts); $display_name = $customer->get_display_name(); $id = $customer->get_id(); $email = $customer->get_email_address(); printf( - '<a href="%s" class="wu-p-1 wu-flex wu-flex-grow wu-rounded wu-items-center wu-no-underline"> - %s - <div class="wu-pl-2"> - <strong class="wu-block">%s <small class="wu-font-normal">(#%s)</small></strong> - <small>%s</small> - </div> - </a>', + '<a href="%s" class="wu-p-1 wu-flex wu-flex-grow wu-rounded wu-items-center wu-no-underline"> + %s + <div class="wu-pl-2"> + <strong class="wu-block">%s <small class="wu-font-normal">(#%s)</small></strong> + <small>%s</small> + </div> + </a>', esc_attr($customer_link), - esc_html($display_name), - get_avatar( - $customer->get_user_id(), + get_avatar( + $customer->get_user_id(), 32, 'identicon', '', [ 'force_display' => true, 'class' => 'wu-rounded-full wu-border-solid wu-border-1 wu-border-white hover:wu-border-gray-400', ] ), + esc_html($display_name), esc_html($id), esc_html($email) ); - break; - case $targets_count > 1: + } else { // $targets_count > 1 foreach ($targets as $key => $target) { $customer = wu_get_customer($target); $tooltip_name = $customer->get_display_name(); $email = $customer->get_email_address(); $url_atts = [ 'id' => $customer->get_id(), ]; $customer_link = wu_network_admin_url('wp-ultimo-edit-customer', $url_atts); printf( "<div class='wu-flex wu--mr-4'><a role='tooltip' aria-label='%s' href='%s'>%s</a></div>", esc_attr($tooltip_name), esc_attr($customer_link), get_avatar( $email, 32, 'identicon', '', [ 'class' => 'wu-rounded-full wu-border-solid wu-border-1 wu-border-white hover:wu-border-gray-400', ] ) ); } if ($targets_count < 7) {inc/admin-pages/class-membership-edit-admin-page.php (1)
1513-1516: Fix: unsafe array_filter indexing can notice/undefined indexarray_filter preserves keys; [0] may not exist. Guard and normalize before indexing.
Apply:
- $existing_quantity = array_filter($membership->get_addon_products(), fn($item) => $item['product']->get_id() === $product->get_id())[0]['quantity']; + $matches = array_filter( + $membership->get_addon_products(), + static fn($item) => isset($item['product']) && $item['product']->get_id() === $product->get_id() + ); + $first = array_values($matches)[0] ?? null; + $existing_quantity = isset($first['quantity']) ? (int) $first['quantity'] : 0;Optionally short-circuit error if $existing_quantity === 0.
inc/checkout/class-legacy-checkout.php (1)
670-677: Undefined method ‘create_account’ will fatal if step handler runsThe hidden “create-account” step points to [$this, 'create_account'] but this class does not define it. Either implement a stub or disable the handler to avoid fatals.
Option A — implement a no-op/stub (until legacy flow is reintroduced):
class Legacy_Checkout { ... + /** + * Legacy create-account handler (stubbed during wp.org review). + */ + protected function create_account(): void { + // Intentionally disabled legacy flow. Keep for BC to avoid fatals. + }Option B — disable handler:
- 'handler' => [$this, 'create_account'], + 'handler' => false,inc/list-tables/class-payment-line-item-list-table.php (1)
89-104: Fix undefined/stale variable in discount/tax tooltips.$tax_rate is conditionally assigned but always interpolated, causing notices and potential cross-leak between sections. Use local vars and safe fallbacks.
- if ($item->get_discount_type() === 'percentage' && $item->get_discount_rate()) { - $tax_rate = $item->get_discount_rate() . '%'; - } - - $tax_label = $item->get_discount_rate() ? ($item->get_discount_label() ?: __('Discount', 'multisite-ultimate')) : __('No discount', 'multisite-ultimate'); - - $tooltip = sprintf('%s (%s)', $tax_rate, $tax_label); + $discount_rate = ''; + if ($item->get_discount_type() === 'percentage' && $item->get_discount_rate()) { + $discount_rate = $item->get_discount_rate() . '%'; + } + $discount_label = $item->get_discount_rate() ? ($item->get_discount_label() ?: __('Discount', 'multisite-ultimate')) : __('No discount', 'multisite-ultimate'); + $tooltip = $discount_rate ? sprintf('%s (%s)', $discount_rate, $discount_label) : $discount_label;- if ($item->get_tax_type() === 'percentage' && $item->get_tax_rate()) { - $tax_rate = $item->get_tax_rate() . '%'; - } - - $tax_label = $item->get_tax_rate() ? ($item->get_tax_label() ?: __('Tax Applied', 'multisite-ultimate')) : __('No Taxes Applied', 'multisite-ultimate'); - - $tooltip = sprintf('%s (%s)', $tax_rate, $tax_label); + $tax_rate_text = ''; + if ($item->get_tax_type() === 'percentage' && $item->get_tax_rate()) { + $tax_rate_text = $item->get_tax_rate() . '%'; + } + $tax_label = $item->get_tax_rate() ? ($item->get_tax_label() ?: __('Tax Applied', 'multisite-ultimate')) : __('No Taxes Applied', 'multisite-ultimate'); + $tooltip = $tax_rate_text ? sprintf('%s (%s)', $tax_rate_text, $tax_label) : $tax_label;Also applies to: 116-131
views/checkout/fields/field-password.php (1)
26-32: Do not pre-populate password fields; remove value attributeEchoing the password into the DOM is a security/UX anti-pattern and unnecessary. Drop the value attribute and let the browser manage autofill.
Apply:
- value="<?php echo esc_attr($field->value); ?>" <?php $field->print_html_attributes(); ?>> + <?php $field->print_html_attributes(); ?>>Optional hardening in inc/ui/class-field.php (avoid accidental value via html_attr and set sane autocomplete if not provided):
@@ public function print_html_attributes(): void { - if ('number' === $this->type) { + if ('number' === $this->type) { ... } + if ('password' === $this->type) { + unset($attributes['value']); + if (empty($attributes['autocomplete'])) { + $attributes['autocomplete'] = 'new-password'; + } + }views/checkout/fields/field-select.php (1)
36-49: Use selected() for instead of checked()checked() won’t select options; use selected() so the current value is correctly reflected.
Apply:
- <option <?php checked(! $field->value); ?> class="wu-opacity-75"><?php echo esc_html($field->placeholder); ?></option> + <option <?php selected(! $field->value); ?> class="wu-opacity-75"><?php echo esc_html($field->placeholder); ?></option> @@ - <?php checked($key, $field->value); ?> + <?php selected($key, $field->value); ?>views/checkout/fields/field-toggle.php (1)
16-21: Escape user-controlled text; drop PHPCS ignoresTitle/tooltip/desc are echoed unescaped. For wp.org review, escape or kses them.
- <?php echo $field->title; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> + <?php echo esc_html( $field->title ); ?> - <?php echo wu_tooltip($field->tooltip); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> + <?php echo wp_kses_post( wu_tooltip( $field->tooltip ) ); ?> - <?php echo $field->desc; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> + <?php echo wp_kses_post( $field->desc ); ?>inc/list-tables/class-discount-code-list-table.php (1)
145-151: Mismatched closing tag: small closed as spanTwo occurrences close with instead of , which breaks markup.
- $html .= '<small class="wu-block">' . sprintf(__('Allowed uses: %d', 'multisite-ultimate'), $item->get_max_uses()) . '</span>'; + $html .= '<small class="wu-block">' . sprintf( __('Allowed uses: %d', 'multisite-ultimate'), $item->get_max_uses() ) . '</small>'; ... - $html .= '<small class="wu-block">' . __('No Limits', 'multisite-ultimate') . '</span>'; + $html .= '<small class="wu-block">' . __( 'No Limits', 'multisite-ultimate' ) . '</small>';inc/ui/class-domain-mapping-element.php (1)
579-595: Form ID mix-up: wrong ID used in “make domain primary” modalThe form in render_user_make_domain_primary_modal() uses the delete modal ID, which can break Vue app wiring and cause collisions.
- $form = new \WP_Ultimo\UI\Form( - 'user_delete_domain_modal', + $form = new \WP_Ultimo\UI\Form( + 'user_make_domain_primary_modal', $fields, [ 'views' => 'admin-pages/fields', @@ - 'html_attr' => [ - 'data-wu-app' => 'user_delete_domain_modal', + 'html_attr' => [ + 'data-wu-app' => 'user_make_domain_primary_modal',inc/ui/class-limits-element.php (1)
247-258: Guard against missing limitation entries to prevent notices.Accessing ->post_types->{$slug}->enabled without existence checks can emit notices for CPTs not present in limitations.
Apply:
- $post_types = array_filter($post_types, fn($post_type_slug) => $this->site->get_limitations()->post_types->{$post_type_slug}->enabled, ARRAY_FILTER_USE_KEY); + $limits = $this->site->get_limitations()->post_types; + $post_types = array_filter( + $post_types, + fn($slug) => isset($limits->{$slug}) && ! empty($limits->{$slug}->enabled), + ARRAY_FILTER_USE_KEY + );views/admin-pages/fields/field-text-display.php (1)
59-64: Add href="#" and role="button" to the copy anchor for keyboard accessibility. Ensure your JS callspreventDefault()to avoid navigation. Confirm that applyingrole="tooltip"on the trigger matches your ARIA model—typicallyrole="tooltip"belongs on the tooltip element, not the trigger.views/admin-pages/fields/field-actions.php (1)
34-38: data-loading target does not exist; ID collision risk.data-loading references wu_action_button_loading_{object_id} but the span id is static. Make the id unique and add basic a11y on the status element.
- <span data-loading="wu_action_button_loading_<?php echo esc_attr($action->object_id); ?>" id="wu_action_button_loading" class="wu-blinking-animation wu-text-gray-600 wu-my-1 wu-text-2xs wu-uppercase wu-font-semibold wu-text-center wu-self-center wu-px-4 wu-py wu-mt-1 hidden" > + <span id="wu_action_button_loading_<?php echo esc_attr($action->object_id); ?>" + data-loading="wu_action_button_loading_<?php echo esc_attr($action->object_id); ?>" + role="status" aria-live="polite" + class="wu-blinking-animation wu-text-gray-600 wu-my-1 wu-text-2xs wu-uppercase wu-font-semibold wu-text-center wu-self-center wu-px-4 wu-py wu-mt-1 hidden">views/checkout/fields/field-text.php (1)
28-36: Mismatched flex container open/close when only suffix is present.The container opens only if prefix, but closes if prefix OR suffix, yielding invalid markup when only suffix exists.
- <?php if ($field->prefix) : ?> - <div class="sm:wu-flex wu-items-stretch wu-content-center"> - <div <?php wu_print_html_attributes($field->prefix_html_attr ?? []); ?>> - <?php echo wp_kses($field->prefix, wu_kses_allowed_html()); ?> - </div> - <?php endif; ?> + <?php if ($field->prefix || $field->suffix) : ?> + <div class="sm:wu-flex wu-items-stretch wu-content-center"> + <?php endif; ?> + <?php if ($field->prefix) : ?> + <div <?php wu_print_html_attributes($field->prefix_html_attr ?? []); ?>> + <?php echo wp_kses($field->prefix, wu_kses_allowed_html()); ?> + </div> + <?php endif; ?> @@ <?php if ($field->prefix || $field->suffix) : ?> </div> <?php endif; ?>Also applies to: 48-52
inc/ui/class-current-membership-element.php (1)
320-333: Avoid array_combine() on empty arrays — it returns falsearray_combine() returns false if either array is empty, which can propagate unexpected false into templates. Use array_column with index to get an empty array instead of false.
Apply:
- $pending_products = array_map( - fn($product) => [ - 'id' => $product['product']->get_id(), - 'quantity' => $product['quantity'], - ], - $swap_membership->get_all_products() - ); - - // add the id as key - $atts['pending_products'] = array_combine(array_column($pending_products, 'id'), $pending_products); + $pending_products = array_map( + fn($product) => [ + 'id' => $product['product']->get_id(), + 'quantity' => $product['quantity'], + ], + $swap_membership->get_all_products() + ); + + // index by id; yields [] when no products (vs false) + $atts['pending_products'] = array_column($pending_products, null, 'id');inc/ui/class-my-sites-element.php (1)
354-361: array_reduce used without an initial accumulator causes fatal errorsBoth reducers assume an array accumulator but omit the initial value. When the input isn’t empty, PHP passes the first element as the initial carry, making
$customer_sites[...]and$user_sites[...]invalid. When empty, result is null. Provide an initial [].Apply:
- $customer_sites = array_reduce( - $this->customer->get_sites(), - function ($customer_sites, $site) { - $customer_sites[ $site->get_id() ] = $site; - return $customer_sites; - } - ); + $customer_sites = array_reduce( + $this->customer->get_sites(), + function ($carry, $site) { + $carry[ $site->get_id() ] = $site; + return $carry; + }, + [] + );- $user_sites = array_reduce( - $wp_user_sites, - function ($user_sites, $wp_site) use ($customer_sites) { - if ( ! array_key_exists($wp_site->userblog_id, $customer_sites ?? []) && get_main_site_id() !== $wp_site->userblog_id) { - $wu_site = wu_get_site($wp_site->userblog_id); - $wu_site->set_membership_id(0); - $user_sites[ $wp_site->userblog_id ] = $wu_site; - } - - return $user_sites; - } - ); + $user_sites = array_reduce( + $wp_user_sites, + function ($carry, $wp_site) use ($customer_sites) { + if ( ! array_key_exists($wp_site->userblog_id, $customer_sites ?? []) && get_main_site_id() !== $wp_site->userblog_id) { + $wu_site = wu_get_site($wp_site->userblog_id); + if ($wu_site) { + $wu_site->set_membership_id(0); + $carry[ $wp_site->userblog_id ] = $wu_site; + } + } + return $carry; + }, + [] + );Also applies to: 366-377
inc/ui/class-base-element.php (1)
968-992: Shortcodes now print instead of return; restore return-by-buffer to prevent broken content and fix display_template()WordPress shortcodes must return strings. After changing
output()to void anddisplay()to not return, shortcode output may appear out of place, anddisplay_template()now operates on null. Capture output indisplay()and return it; echo it at call sites that expect side-effects.Apply:
- /** - * Treats the attributes before passing them down to the output method. - * - * @since 2.0.0 - * - * @param array $atts The element attributes. - * @return void - */ - public function display($atts) { + /** + * Treats the attributes before passing them down to the output method. + * + * @since 2.0.0 + * + * @param array $atts The element attributes. + * @return string Rendered HTML for shortcode/block contexts. + */ + public function display($atts) { if ( ! $this->should_display()) { - return; // bail if the display was set to false. + return ''; } $this->dependencies(); $atts = wp_parse_args($atts, $this->defaults()); /* * Account for the 'className' Gutenberg attribute. */ $atts['className'] = trim('wu-' . $this->id . ' wu-element ' . wu_get_isset($atts, 'className', '')); /* * Pass down the element so we can use helpers. */ $atts['element'] = $this; - call_user_func([$this, 'output'], $atts); + ob_start(); + $this->output($atts); + return (string) ob_get_clean(); }- $this->display($atts); + echo $this->display($atts);- $this->display($atts); + echo $this->display($atts);Also applies to: 1096-1100, 1177-1181
inc/ui/class-checkout-element.php (1)
345-360: Broken link and improper escaping in translatable stringesc_html__() escapes the and esc_attr() is not the right sanitizer for href. Use wp_kses + esc_url and print the line separately.
Apply:
-echo '<br>' . sprintf(esc_html__('Click <a href="%s">here</a> to pay.', 'multisite-ultimate'), esc_attr($payment_url)); +echo '<br>'; +printf( + wp_kses( + __('Click <a href="%s">here</a> to pay.', 'multisite-ultimate'), + array( 'a' => array( 'href' => array() ) ) + ), + esc_url($payment_url) +);
| printf( | ||
| '<div class="wu-bg-gray-100 wu-p-2 wu-text-right wu-border-0 wu-border-b wu-border-solid wu-border-gray-400"> | ||
| <a href="#" attr-slug-csv="%2$s" class="wu-export-button wu-no-underline wu-text-gray-800 wu-text-xs"> | ||
| <span class="dashicons-wu-download wu-mr-1"></span> %1$s | ||
| </a> | ||
| <input type="hidden" id="csv_headers_%2$s" value="%3$s" /> | ||
| <input type="hidden" id="csv_data_%2$s" value="%4$s" /> | ||
| <input type="hidden" id="csv_action_%2$s" value="%5$s" /> | ||
| </div>'; | ||
|
|
||
| $html = apply_filters('wu_export_html_render', $html, $html); | ||
|
|
||
| printf( | ||
| $html, // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
| apply_filters('wu_export_data_table_label', esc_html__('CSV', 'multisite-ultimate')), // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
| </div>', | ||
| esc_html__('CSV', 'multisite-ultimate'), | ||
| esc_attr($slug), |
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.
💡 Verification agent
🧩 Analysis chain
Use data- attributes and keep label filter for backward compatibility.*
Custom attributes should be data-* per HTML5. Also, hard-coding the label drops the prior customization point.
Apply this diff:
- printf(
- '<div class="wu-bg-gray-100 wu-p-2 wu-text-right wu-border-0 wu-border-b wu-border-solid wu-border-gray-400">
- <a href="#" attr-slug-csv="%2$s" class="wu-export-button wu-no-underline wu-text-gray-800 wu-text-xs">
- <span class="dashicons-wu-download wu-mr-1"></span> %1$s
+ printf(
+ '<div class="wu-bg-gray-100 wu-p-2 wu-text-right wu-border-0 wu-border-b wu-border-solid wu-border-gray-400">
+ <a href="#" data-slug-csv="%2$s" class="wu-export-button wu-no-underline wu-text-gray-800 wu-text-xs">
+ <span class="dashicons-wu-download wu-mr-1"></span> %1$s
</a>
<input type="hidden" id="csv_headers_%2$s" value="%3$s" />
<input type="hidden" id="csv_data_%2$s" value="%4$s" />
<input type="hidden" id="csv_action_%2$s" value="%5$s" />
</div>',
- esc_html__('CSV', 'multisite-ultimate'),
+ esc_html(apply_filters('wu_export_data_table_label', __('CSV', 'multisite-ultimate'))),
esc_attr($slug),
esc_attr($header_strings),
esc_attr($data_strings),
esc_attr($args['action'])
);If JS reads the old attribute, update it:
🏁 Script executed:
#!/bin/bash
# Update JS to read data-slug-csv instead of attr-slug-csv
rg -n 'attr-slug-csv'Length of output: 4278
Use HTML5 data-attributes and enable CSV label filtering
- In inc/admin-pages/class-dashboard-admin-page.php (≈ line 613), change
attr-slug-csv="%2$s"todata-slug-csv="%2$s"and replace the hard-coded label%1$swith:
esc_html( apply_filters( 'wu_export_data_table_label', __( 'CSV', 'multisite-ultimate' ) ) ) - In assets/js/dashboard-statistics.js (line 188), update
getAttribute('attr-slug-csv')togetAttribute('data-slug-csv')and recompile assets (to regenerate dashboard-statistics.min.js).
🤖 Prompt for AI Agents
In inc/admin-pages/class-dashboard-admin-page.php around lines 611 to 621,
change the non-standard attribute attr-slug-csv to the HTML5 data attribute
data-slug-csv and replace the hard-coded CSV label with a filtered label by
using esc_html( apply_filters( 'wu_export_data_table_label', __( 'CSV',
'multisite-ultimate' ) ) ); also update assets/js/dashboard-statistics.js
(around line 188) to read the attribute via getAttribute('data-slug-csv')
instead of getAttribute('attr-slug-csv') and then recompile/rebuild assets to
regenerate dashboard-statistics.min.js so the frontend uses the new data
attribute and filterable label.
| 'before' => fn() => null, | ||
| 'after' => fn() => null, | ||
| 'title' => __('List Table', 'multisite-ultimate'), |
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.
Potential fatal: defaulting 'before'/'after' to Closures may break templates expecting strings.
If widget-list-table template echoes these keys, echoing a Closure triggers “Object of class Closure could not be converted to string.” Either revert defaults to strings, or ensure templates treat them as callable-or-string.
Minimal safe fix (consistent with other methods in this class):
- 'before' => fn() => null,
- 'after' => fn() => null,
+ 'before' => '',
+ 'after' => '',Alternatively, normalize just before rendering:
$before = $atts['before'];
$after = $atts['after'];
$render = static fn($seg) => is_callable($seg) ? $seg() : print $seg;
...
// in the meta box callback, use $render($before); ... $render($after);🤖 Prompt for AI Agents
In inc/admin-pages/class-edit-admin-page.php around lines 404 to 406, the
default values for 'before' and 'after' are set to Closures which can cause a
fatal "Object of class Closure could not be converted to string" if templates
echo these keys; change the defaults to empty strings (or the same string
defaults used elsewhere in this class) so templates that expect strings won't
break, or alternatively add normalization in the render path that checks
is_callable($seg) and calls it, otherwise echoes/prints the string before/after
rendering.
| ob_start(); | ||
| wu_tooltip(__('Just like this!', 'multisite-ultimate')); | ||
| $realtooltip = ob_end_clean(); | ||
|
|
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.
Fix: buffer is discarded; use ob_get_clean() to capture tooltip HTML
ob_end_clean() drops the buffer and returns bool, so $realtooltip becomes truthy/empty instead of HTML. Use ob_get_clean().
Apply:
- ob_start();
- wu_tooltip(__('Just like this!', 'multisite-ultimate'));
- $realtooltip = ob_end_clean();
+ ob_start();
+ wu_tooltip(__('Just like this!', 'multisite-ultimate'));
+ $realtooltip = ob_get_clean();📝 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.
| ob_start(); | |
| wu_tooltip(__('Just like this!', 'multisite-ultimate')); | |
| $realtooltip = ob_end_clean(); | |
| ob_start(); | |
| wu_tooltip(__('Just like this!', 'multisite-ultimate')); | |
| $realtooltip = ob_get_clean(); |
🤖 Prompt for AI Agents
In inc/checkout/signup-fields/class-base-signup-field.php around lines 479 to
482, the output buffer is being ended with ob_end_clean() which discards the
buffer and returns a boolean, so $realtooltip does not contain the tooltip HTML;
replace the ob_end_clean() call with ob_get_clean() so the buffered tooltip HTML
is captured into $realtooltip (i.e., start output buffering, call
wu_tooltip(...), then assign the buffer content to $realtooltip via
ob_get_clean()).
| * @return void | ||
| */ | ||
| public static function tooltip($text, $icon = 'dashicons-editor-help') { | ||
|
|
||
| _deprecated_function(__METHOD__, '2.0.0', 'wu_tooltip()'); | ||
|
|
||
| return wu_tooltip($text, $icon); | ||
| wu_tooltip($text, $icon); | ||
| } |
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.
💡 Verification agent
🧩 Analysis chain
Preserve BC: return buffered HTML instead of printing directly
Changing this deprecated shim to print-only risks breaking third‑party code that still expects a string return (e.g., concatenation or assignment). Capture the printed output from wu_tooltip() and return it; keep not echoing here to avoid double output when callers do echo WU_Util::tooltip(...).
Apply this diff:
- * @return void
+ * @return string
...
- public static function tooltip($text, $icon = 'dashicons-editor-help') {
+ public static function tooltip($text, $icon = 'dashicons-editor-help') {
_deprecated_function(__METHOD__, '2.0.0', 'wu_tooltip()');
-
- wu_tooltip($text, $icon);
+ ob_start();
+ wu_tooltip($text, $icon);
+ return ob_get_clean();
}Run to find any usages relying on a return value:
🏁 Script executed:
#!/bin/bash
# Assignments or concatenations using WU_Util::tooltip(...)
rg -nP --type=php -C2 $'=\s*WU_Util::tooltip\s*\('
rg -nP --type=php -C2 $'\.\s*WU_Util::tooltip\s*\('
# Direct echo (safe if we keep returning a string and not echo in the shim)
rg -nP --type=php -C2 $'echo\s+WU_Util::tooltip\s*\('Length of output: 1751
Capture and return tooltip HTML while preserving existing echo behavior
Update the deprecated shim to buffer wu_tooltip(), echo its output (so direct calls still print) and return the HTML string for assignments.
Apply this diff in inc/deprecated/deprecated.php (at the existing tooltip method):
- * @return void
+ * @return string
...
- public static function tooltip($text, $icon = 'dashicons-editor-help') {
+ public static function tooltip($text, $icon = 'dashicons-editor-help') {
_deprecated_function(__METHOD__, '2.0.0', 'wu_tooltip()');
- wu_tooltip($text, $icon);
+ ob_start();
+ wu_tooltip($text, $icon);
+ $html = ob_get_clean();
+ echo $html;
+ return $html;
}📝 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.
| * @return void | |
| */ | |
| public static function tooltip($text, $icon = 'dashicons-editor-help') { | |
| _deprecated_function(__METHOD__, '2.0.0', 'wu_tooltip()'); | |
| return wu_tooltip($text, $icon); | |
| wu_tooltip($text, $icon); | |
| } | |
| /** | |
| * @return string | |
| */ | |
| public static function tooltip($text, $icon = 'dashicons-editor-help') { | |
| _deprecated_function(__METHOD__, '2.0.0', 'wu_tooltip()'); | |
| ob_start(); | |
| wu_tooltip($text, $icon); | |
| $html = ob_get_clean(); | |
| echo $html; | |
| return $html; | |
| } |
🤖 Prompt for AI Agents
In inc/deprecated/deprecated.php around lines 506 to 513, the deprecated tooltip
shim currently calls wu_tooltip() but does not return its HTML; update the
method to start output buffering before calling wu_tooltip(), capture the
buffered HTML into a variable, end/clean the buffer, echo the HTML (to preserve
existing direct-output behavior), and then return the HTML string; keep the
existing _deprecated_function() call and signature unchanged.
|
|
||
| ?> | ||
| <div class="<?php echo esc_attr(trim($field->wrapper_classes)); ?>" <?php echo $field->get_wrapper_html_attributes(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>> | ||
| <div class="<?php echo esc_attr(trim($field->wrapper_classes)); ?>" <?php $field->print_wrapper_html_attributes(); ?>> |
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.
Potential duplicate class attribute due to print_wrapper_html_attributes bug
Using print_wrapper_html_attributes here is correct, but the current implementation unsets 'class' on the object property, not on the local copy it prints, so 'class' is still emitted by wu_print_html_attributes. This can produce duplicate class attributes in this div.
Apply this fix in inc/ui/class-field.php:
public function print_wrapper_html_attributes(): void {
-
- $attributes = $this->atts['wrapper_html_attr'];
-
- unset($this->atts['wrapper_html_attr']['class']);
-
- wu_print_html_attributes($attributes);
+ $attributes = $this->atts['wrapper_html_attr'] ?? [];
+ unset($attributes['class']); // avoid duplicate class on wrappers
+ wu_print_html_attributes($attributes);
}📝 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.
| <div class="<?php echo esc_attr(trim($field->wrapper_classes)); ?>" <?php $field->print_wrapper_html_attributes(); ?>> | |
| public function print_wrapper_html_attributes(): void { | |
| // Safely grab any wrapper attributes, or default to an empty array | |
| $attributes = $this->atts['wrapper_html_attr'] ?? []; | |
| // Remove 'class' here so wu_print_html_attributes() doesn’t emit a duplicate | |
| unset($attributes['class']); | |
| wu_print_html_attributes($attributes); | |
| } |
🤖 Prompt for AI Agents
views/checkout/fields/field-products.php lines 10-10: the div echoes
wrapper_classes and then calls print_wrapper_html_attributes which currently
unsets 'class' on the object property instead of the local attributes it prints,
causing duplicate class attributes; fix inc/ui/class-field.php so
print_wrapper_html_attributes builds a local attributes array (or clones the
attributes), remove/unset the 'class' key only from that local array (not from
the object property), then pass the cleaned local array to
wu_print_html_attributes (ensuring proper escaping) so the class attribute is
only emitted once.
| <?php else : ?> | ||
|
|
||
| </<?php echo $form->wrap_tag; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>> | ||
| </<?php esc_attr($form->wrap_tag); ?>> |
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.
Broken closing tag — missing echo.
The closing tag does not echo wrap_tag, producing </> and breaking the DOM.
Apply:
- </<?php esc_attr($form->wrap_tag); ?>>
+ </<?php echo esc_attr($form->wrap_tag); ?>>📝 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.
| </<?php esc_attr($form->wrap_tag); ?>> | |
| </<?php echo esc_attr($form->wrap_tag); ?>> |
🤖 Prompt for AI Agents
In views/checkout/fields/form.php around line 34, the closing tag currently
outputs an empty tag (</>) because it doesn't echo the wrapper tag; replace it
so the PHP prints the escaped wrap_tag (use <?php echo
esc_attr($form->wrap_tag); ?> inside the closing angle brackets) ensuring proper
PHP echo and semicolon so the DOM is closed as </wrap_tag>.
| <a href="mailto:<?php echo esc_attr($user->user_email); ?>" class="wu-no-underline" <?php wu_tooltip_text(esc_html__('Send an email to this customer.', 'multisite-ultimate')); ?>> | ||
| <?php echo esc_html($user->user_email); ?> | ||
| </a> |
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.
🛠️ Refactor suggestion
Avoid double-escaping aria-label text
wu_tooltip_text() already escapes via esc_attr(). Passing esc_html__() causes entities to double-escape.
Apply:
- <a href="mailto:<?php echo esc_attr($user->user_email); ?>" class="wu-no-underline" <?php wu_tooltip_text(esc_html__('Send an email to this customer.', 'multisite-ultimate')); ?>>
+ <a href="mailto:<?php echo esc_attr($user->user_email); ?>" class="wu-no-underline" <?php wu_tooltip_text(__('Send an email to this customer.', 'multisite-ultimate')); ?>>🤖 Prompt for AI Agents
In views/customers/widget-avatar.php around lines 56 to 58, the tooltip text is
being double-escaped because wu_tooltip_text() already calls esc_attr(); replace
esc_html__('Send an email to this customer.', 'multisite-ultimate') with the
unescaped translation call __('Send an email to this customer.',
'multisite-ultimate') so wu_tooltip_text() can perform the escaping once.
| <button class="button wu-w-full" disabled="disabled"> | ||
| <span <?php echo wu_tooltip_text(esc_html__('Switching to your own account is not possible.', 'multisite-ultimate')); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>> | ||
| <span <?php wu_tooltip_text(esc_html__('Switching to your own account is not possible.', 'multisite-ultimate')); ?>> | ||
| <?php esc_html_e('Switch To →', 'multisite-ultimate'); ?> | ||
| </span> |
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.
🛠️ Refactor suggestion
Avoid double-escaping aria-label text (same issue)
Pass raw translation; wu_tooltip_text() handles escaping.
Apply:
- <span <?php wu_tooltip_text(esc_html__('Switching to your own account is not possible.', 'multisite-ultimate')); ?>>
+ <span <?php wu_tooltip_text(__('Switching to your own account is not possible.', 'multisite-ultimate')); ?>>📝 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.
| <button class="button wu-w-full" disabled="disabled"> | |
| <span <?php echo wu_tooltip_text(esc_html__('Switching to your own account is not possible.', 'multisite-ultimate')); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>> | |
| <span <?php wu_tooltip_text(esc_html__('Switching to your own account is not possible.', 'multisite-ultimate')); ?>> | |
| <?php esc_html_e('Switch To →', 'multisite-ultimate'); ?> | |
| </span> | |
| <button class="button wu-w-full" disabled="disabled"> | |
| <span <?php wu_tooltip_text(__('Switching to your own account is not possible.', 'multisite-ultimate')); ?>> | |
| <?php esc_html_e('Switch To →', 'multisite-ultimate'); ?> | |
| </span> |
🤖 Prompt for AI Agents
In views/customers/widget-avatar.php around lines 76 to 79, the tooltip text is
being double-escaped by passing esc_html__ to wu_tooltip_text(); change the
tooltip call to pass the raw translation function (use __ with the same
text/domain) so wu_tooltip_text() can perform the escaping itself, leaving the
visible button label (esc_html_e) unchanged.
| '<span class="wu-flag-icon wu-w-5 wu-mr-1" role="tooltip" aria-label="%s">%s</span>', | ||
| esc_html(wu_get_country_name($country_code)), | ||
| esc_html(wu_get_flag_emoji($country_code)), | ||
| ); |
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.
🛠️ Refactor suggestion
Fix ARIA semantics on flag span (don’t use role="tooltip").
role="tooltip" is for the tooltip element itself, not the icon. Since the country name is printed right after the emoji, treat the emoji as decorative to avoid duplicate announcements.
Apply this diff:
- '<span class="wu-flag-icon wu-w-5 wu-mr-1" role="tooltip" aria-label="%s">%s</span>',
- esc_html(wu_get_country_name($country_code)),
+ '<span class="wu-flag-icon wu-w-5 wu-mr-1" aria-hidden="true">%s</span>',
esc_html(wu_get_flag_emoji($country_code)),Alternative: if you want the emoji to convey the country name itself, use role="img" with aria-label and remove the visible country name next to it to avoid duplication.
📝 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.
| '<span class="wu-flag-icon wu-w-5 wu-mr-1" role="tooltip" aria-label="%s">%s</span>', | |
| esc_html(wu_get_country_name($country_code)), | |
| esc_html(wu_get_flag_emoji($country_code)), | |
| ); | |
| '<span class="wu-flag-icon wu-w-5 wu-mr-1" aria-hidden="true">%s</span>', | |
| esc_html(wu_get_flag_emoji($country_code)), |
🤖 Prompt for AI Agents
In views/dashboard-statistics/widget-countries.php around lines 68–71, the span
for the flag currently uses role="tooltip" which is incorrect; treat the emoji
as decorative to avoid duplicate announcements by removing role="tooltip" and
adding aria-hidden="true" to the flag span so screen readers ignore the emoji
while the visible country name remains accessible; alternatively, if you prefer
the emoji to serve as the accessible label, replace role="tooltip" with
role="img", add aria-label with the country name and remove the visible text
country name to avoid duplication.
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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
views/checkout/templates/template-selection/legacy.php (1)
209-215: Anchor can render without href; add rel=noopener; avoid ternary with side effects.When previews are enabled, the anchor only gets onclick from get_preview_url_attrs(), leaving no href (poor a11y/keyboard, JS-off). Also, the fallback branch opens a new tab without rel="noopener" and uses esc_attr for a URL. Prefer an if/else, always include href, and use esc_url.
Apply:
- <?php $is_template ? $site->get_preview_url_attrs() : printf('href="%s" target="_blank"', esc_attr($site->get_active_site_url())); ?> + <?php if ($is_template) { + $site->get_preview_url_attrs(); + } else { + printf('href="%s" target="_blank" rel="noopener"', esc_url($site->get_active_site_url())); + } ?>views/legacy/signup/pricing-table/plan.php (1)
115-121: Use esc_url() for href.Sanitize the URL properly to prevent attribute injection.
- <a href="<?php echo esc_attr($plan->contact_us_link); ?>" class="button button-primary"> + <a href="<?php echo esc_url($plan->contact_us_link); ?>" class="button button-primary">views/dashboard-widgets/thank-you.php (2)
257-257: Escape URLs with esc_url and add rel=noopener for target=_blank.Use esc_url for href/src (protocol validation) instead of esc_attr, and add rel="noopener" on the external target.
- src="<?php echo esc_attr($site->get_featured_image('thumbnail')); ?>" + src="<?php echo esc_url($site->get_featured_image('thumbnail')); ?>" - <a v-if="!creating" href="<?php echo esc_attr(wu_get_current_url()); ?>" class="wu-block sm:wu-inline-block wu-no-underline"> + <a v-if="!creating" href="<?php echo esc_url(wu_get_current_url()); ?>" class="wu-block sm:wu-inline-block wu-no-underline"> - <a href="<?php echo esc_attr(get_admin_url($site->get_id())); ?>" class="wu-block sm:wu-inline-block wu-no-underline sm:wu-mr-4"> + <a href="<?php echo esc_url(get_admin_url($site->get_id())); ?>" class="wu-block sm:wu-inline-block wu-no-underline sm:wu-mr-4"> - <a href="<?php echo esc_attr(wu_with_sso(get_site_url($site->get_id()))); ?>" class="wu-block sm:wu-inline-block wu-no-underline" target="_blank"> + <a href="<?php echo esc_url(wu_with_sso(get_site_url($site->get_id()))); ?>" class="wu-block sm:wu-inline-block wu-no-underline" target="_blank" rel="noopener">Also applies to: 304-304, 315-315, 320-320
37-39: Sanitize shortcode output.Shortcodes can return HTML; sanitize with wp_kses(..., wu_kses_allowed_html()) before echoing.
- <?php echo do_shortcode($thank_you_message); ?> + <?php echo wp_kses(do_shortcode($thank_you_message), wu_kses_allowed_html()); ?> - <?php echo do_shortcode($thank_you_message_pending); ?> + <?php echo wp_kses(do_shortcode($thank_you_message_pending), wu_kses_allowed_html()); ?> - <?php echo do_shortcode($no_sites_message); ?> + <?php echo wp_kses(do_shortcode($no_sites_message), wu_kses_allowed_html()); ?>Also applies to: 133-135, 337-339
views/wizards/host-integrations/activation.php (1)
103-104: Use esc_url() for href attribute
The “Continue” link escapes with esc_attr(), not esc_url(); this can break URLs and weaken validation.Apply:
- <a href="<?php echo esc_attr($page->get_next_section_link()); ?>" class="button button-large"> + <a href="<?php echo esc_url($page->get_next_section_link()); ?>" class="button button-large">views/checkout/fields/field-select.php (2)
29-35: Remove non-standard value attribute on . HTML select doesn’t support value; selection is controlled by option[selected]. -<select +<select class="form-control wu-w-full wu-my-1 <?php echo esc_attr(trim($field->classes)); ?>" id="field-<?php echo esc_attr($field->id); ?>" name="<?php echo esc_attr($field->id); ?>" - value="<?php echo esc_attr($field->value); ?>" <?php $field->print_html_attributes(); ?> > 37-41: Use selected() for , not checked(). Add empty value for placeholder. Using checked() yields invalid markup and won’t select correctly. -<?php if ($field->placeholder) : ?> - <option <?php checked(! $field->value); ?> class="wu-opacity-75"><?php echo esc_html($field->placeholder); ?></option> +<?php if ($field->placeholder) : ?> + <option value="" <?php selected(empty($field->value)); ?> class="wu-opacity-75"><?php echo esc_html($field->placeholder); ?></option> <?php endif; ?> ... - <option - value="<?php echo esc_attr($key); ?>" - <?php checked($key, $field->value); ?> - > + <option + value="<?php echo esc_attr($key); ?>" + <?php selected($key, $field->value); ?> + > <?php echo esc_html($label); ?> </option> Also applies to: 45-50 inc/ui/class-field.php (2) 216-224: Deprecated-type check uses array_key_exists on a list; should use in_array. This condition never matches because $deprecated is a numeric array. Apply this diff: - if (array_key_exists($this->type, $deprecated)) { + if (in_array($this->type, $deprecated, true)) { 408-414: Number bounds ignore 0; use strict false checks. Truthiness makes min=0/max=0 ineffective. Apply this diff: - if ($this->min && $value < $this->min) { + if (false !== $this->min && $value < $this->min) { return $this->min; } - if ($this->max && $value > $this->max) { + if (false !== $this->max && $value > $this->max) { return $this->max; } ♻️ Duplicate comments (3) inc/list-tables/class-base-list-table.php (1) 838-839: Escape aria-label; consider escaping inner text (matches prior review) Escape the aria-label value with esc_attr(). Optionally, avoid injecting raw HTML via $text; print escaped parts explicitly. Minimal fix: - return sprintf('<span role="tooltip" aria-label="%s">%s</span>', date_i18n('Y-m-d H:i:s', $time), $text); + return sprintf('<span role="tooltip" aria-label="%s">%s</span>', esc_attr( date_i18n('Y-m-d H:i:s', $time) ), $text); Optional harderening: - $text = $formatted_value . sprintf('<br><small>%s</small>', sprintf($placeholder, human_time_diff($time))); - return sprintf('<span role="tooltip" aria-label="%s">%s</span>', esc_attr( date_i18n('Y-m-d H:i:s', $time) ), $text); + $rel = sprintf($placeholder, human_time_diff($time)); + return sprintf( + '<span role="tooltip" aria-label="%s">%s<br><small>%s</small></span>', + esc_attr( date_i18n('Y-m-d H:i:s', $time) ), + esc_html( $formatted_value ), + esc_html( $rel ) + ); views/checkout/fields/form.php (1) 35-35: Broken closing tag — missing echo. Still outputs </> instead of the actual wrapper tag. - </<?php esc_attr($form->wrap_tag); ?>> + </<?php echo esc_attr($form->wrap_tag); ?>> views/admin-pages/fields/form.php (1) 23-23: Prevent duplicate id/action/method on . Calling Form::print_html_attributes() after hardcoding id/action/method can duplicate them. Please ensure Form::print_html_attributes() unsets: id, action, method. 🧹 Nitpick comments (20) views/shortcodes/shortcodes.php (2) 38-38: Confirm content expectations for descriptions; optionally allow safe HTML or guard missing index If descriptions may include basic markup (links, code), esc_html will neuter it. If plain text only, keep as-is but guard against undefined index. Option A — allow limited HTML safely: - <?php echo esc_html($shortcode['description']); ?> + <?php echo wp_kses_post( $shortcode['description'] ?? '' ); ?> Option B — keep plain text but avoid notices: - <?php echo esc_html($shortcode['description']); ?> + <?php echo esc_html( $shortcode['description'] ?? '' ); ?> 65-65: Same consideration for parameter descriptions If parameter descriptions can contain inline HTML, prefer wp_kses_post; otherwise keep esc_html with a null-coalescing guard. Option A: - <?php echo esc_html($value['desc']); ?> + <?php echo wp_kses_post( $value['desc'] ?? '' ); ?> Option B: - <?php echo esc_html($value['desc']); ?> + <?php echo esc_html( $value['desc'] ?? '' ); ?> views/legacy/signup/signup-nav-links.php (2) 53-53: Confirm intent to drop inline HTML in labels; otherwise allowlist safe tags Switching to esc_html() neutralizes the intended in the default “Log In” label and any markup provided via the filter. If you still want bold while keeping safety, allowlist only what you need. - <?php echo esc_html($label); ?> + <?php echo wp_kses($label, ['strong' => []]); ?> If the goal is to forbid all HTML from third parties, keep esc_html() and remove the from the default label above for consistency. 49-70: Nit: avoid repeated count() and/or simplify separator rendering Minor perf/readability: compute count() once, or build the anchors and implode with ' | '. Option A (minimal change): - <?php $i = 1; foreach ($nav_links as $link => $label) : ?> + <?php $i = 1; $total = count($nav_links); foreach ($nav_links as $link => $label) : ?> ... - if ($i < count($nav_links)) { + if ($i < $total) { Option B (simpler rendering): - <?php $i = 1; foreach ($nav_links as $link => $label) : ?> - <a href="<?php echo esc_attr($link); ?>"> - <?php echo esc_html($label); ?> - </a> - <?php if ($i < count($nav_links)) { echo ' | '; ++$i; } ?> - <?php endforeach; ?> + <?php + $links_html = array_map( + static function ($link, $label) { + return sprintf( + '<a href="%s">%s</a>', + esc_url($link), + wp_kses($label, ['strong' => []]) + ); + }, + array_keys($nav_links), + $nav_links + ); + echo implode(' | ', $links_html); + ?> views/legacy/signup/pricing-table/plan.php (2) 34-34: Escape the entire class attribute once; avoid mixing raw/escaped fragments. Build the class list then escape as a whole to reduce risk and improve readability. - class="<?php echo esc_attr("wu-product-{$plan->get_id()}"); ?> lift wu-plan plan-tier <?php echo $plan->is_featured_plan() ? 'callout' : ''; ?> wu-col-sm-<?php echo esc_attr($columns); ?> wu-col-xs-12"> +<?php + $classes = [ + "wu-product-{$plan->get_id()}", + 'lift', + 'wu-plan', + 'plan-tier', + $plan->is_featured_plan() ? 'callout' : '', + "wu-col-sm-{$columns}", + 'wu-col-xs-12', + ]; +?> + class="<?php echo esc_attr(trim(implode(' ', array_filter($classes)))); ?>"> 110-113: Remove unused $button_attrubutes (and/or reinstate sanitized support); strengthen comparisons. The filter return is discarded and the var name is misspelled. Either drop the hook intentionally or print sanitized attributes. Also prefer strict comparisons. -$button_attrubutes = apply_filters('wu_plan_select_button_attributes', '', $plan, $current_plan); -$button_label = null != $current_plan && $plan->get_id() == $current_plan->id ? __('This is your current plan', 'multisite-ultimate') : __('Select Plan', 'multisite-ultimate'); +$button_label = null !== $current_plan && $plan->get_id() === $current_plan->id ? __('This is your current plan', 'multisite-ultimate') : __('Select Plan', 'multisite-ultimate'); $button_label = apply_filters('wu_plan_select_button_label', $button_label, $plan, $current_plan); If you want to keep extensibility, consider changing the hook to return an array of attributes and print them via printf(' %s="%s"', esc_attr($k), esc_attr($v));. views/dashboard-widgets/thank-you.php (1) 227-235: Remove tautological conditions. if ('Site') and if ('Order Details') are always true; drop the wrappers. - <?php if ('Site') : ?> <h4 class="wu-m-0 <?php echo esc_attr(wu_env_picker('', 'wu-widget-title')); ?>"> <?php esc_html_e('Site', 'multisite-ultimate'); ?> </h4> - <?php endif; ?> - <?php if ('Order Details') : ?> <h4 class="wu-m-0 <?php echo esc_attr(wu_env_picker('', 'wu-widget-title')); ?>"> <?php esc_html_e('Order Details', 'multisite-ultimate'); ?> </h4> - <?php endif; ?> Also applies to: 357-365 views/dashboard-statistics/widget-revenue.php (1) 14-16: Tooltip ARIA semantics: consider aria-describedby instead of role=tooltip on the li. role="tooltip" should be on the tooltip element itself, with the trigger referencing it via aria-describedby. Consider updating wu_tooltip_text(...) to output aria-describedby="" and render a hidden tooltip node with that id. I can draft a small helper and markup pattern if you want. inc/functions/helper.php (1) 400-407: SVG attribute casing robustness. Include both camelCase and lowercase variants for viewBox/preserveAspectRatio to avoid client/parser quirks. views/dashboard-widgets/current-membership-product-details.php (1) 61-61: Harden class sanitization and avoid recomputing KSES in loop Use sanitize_html_class() for the dynamic class. Cache the allowlist outside the foreach to avoid repeated function calls. If $line may contain user/remote content, prefer a stricter allowlist (e.g., wp_kses_post()) instead of wu_kses_allowed_html(), which currently permits Vue/data attributes. - <?php foreach ($product->get_pricing_table_lines() as $key => $line) : ?> + <?php $allowed = wu_kses_allowed_html(); ?> + <?php foreach ($product->get_pricing_table_lines() as $key => $line) : ?> - <li class="<?php echo esc_attr(str_replace('_', '-', $key)); ?>"><?php echo wp_kses($line, wu_kses_allowed_html()); ?></li> + <li class="<?php echo esc_attr( sanitize_html_class( str_replace('_', '-', $key) ) ); ?>"> + <?php echo wp_kses($line, $allowed); ?> + </li> views/base/edit/widget-tabs.php (1) 137-137: Sanitizing $after is correct; prefer caching allowlist if reused If wu_kses_allowed_html() is called multiple times in the render path, cache it once and reuse. - <?php echo wp_kses($after, wu_kses_allowed_html()); ?> + <?php static $allowed; $allowed = $allowed ?? wu_kses_allowed_html(); echo wp_kses($after, $allowed); ?> If $after can include user-provided fragments, consider a stricter allowlist (wp_kses_post()) to align with wp.org expectations. views/checkout/fields/form.php (1) 8-8: Fix the docblock variable name. Should document $form, not $field. -/** @var $field \WP_Ultimo\UI\Form */ +/** @var \WP_Ultimo\UI\Form $form */ views/admin-pages/fields/field-group.php (1) 43-46: Avoid early escaping in data assignment. Escaping should happen at render time to prevent double-encoding. - 'classes' => trim('wu-flex ' . esc_attr($field->classes)), + 'classes' => trim('wu-flex ' . $field->classes), views/admin-pages/fields/field-html.php (1) 29-36: Correct the comment label. This block loads the description partial, not the title. - /** - * Adds the partial title template. + /** + * Adds the partial description template. views/settings/fields/field-text.php (1) 25-25: Eliminate potential duplicate attributes on .
print_html_attributes() may output id/name/type/value/placeholder/disabled already; combined with explicit attrs here can duplicate.Two options:
- Preferred: Ensure Field::print_html_attributes() unsets these keys.
- Or: rely solely on the printer and remove explicit duplicates here.
Proposed core change (inc/ui/class-field.php::print_html_attributes):
- unset($this->atts['html_attr']['class']); - $attributes = $this->atts['html_attr']; + $attributes = $this->atts['html_attr'] ?? []; + unset( + $attributes['class'], + $attributes['id'], + $attributes['name'], + $attributes['type'], + $attributes['value'], + $attributes['placeholder'], + $attributes['disabled'] + );inc/ui/class-field.php (5)
18-64: Docblock types are inconsistent with actual usage (options/args/rules/min/max/stacked/disabled/money).Align types to reflect reality and aid IDEs and static analysis.
Apply this diff:
- * @property string|false args + * @property array|callable|false args ... - * @property string|false options + * @property array|callable|false options ... - * @property string|false options_template + * @property string|false options_template ... - * @property string|false rules + * @property array|false rules ... - * @property int|false min + * @property int|float|false min - * @property int|false max + * @property int|float|false max ... - * @property string|false money + * @property bool|array money - * @property string|false stacked If the field is inside a restricted container + * @property bool stacked If the field is inside a restricted container ... - * @property string|false disabled + * @property bool|string disabled
472-476: Nit: “Echos” → “Echoes”.Minor grammar in the docblock.
Apply this diff:
- * Echos HTML attributes for the field. + * Echoes HTML attributes for the field.
464-466: Potential v-bind collision.Setting $attributes['v-bind'] unconditionally may overwrite an existing Vue binding. Consider merging or using a dedicated key (e.g., data-money-settings) and letting Vue pick it up.
Would you like a follow-up patch to merge existing v-bind objects if present?
436-441: BC strategy for template callers.Since these methods now print, older templates may rely on returned strings. Optionally keep deprecated getters that buffer output.
Example:
/** @deprecated 2.4.4 Use print_html_attributes() */ public function get_html_attributes(): string { ob_start(); $this->print_html_attributes(); return (string) ob_get_clean(); } /** @deprecated 2.4.4 Use print_wrapper_html_attributes() */ public function get_wrapper_html_attributes(): string { ob_start(); $this->print_wrapper_html_attributes(); return (string) ob_get_clean(); }Also applies to: 472-476
436-469: Localize and normalizehtml_attrinstead of mutating$this->atts
Apply this diff in inc/ui/class-field.php:- if (is_callable($this->atts['html_attr'])) { - $this->atts['html_attr'] = call_user_func($this->atts['html_attr']); - } - - unset($this->atts['html_attr']['class']); - $attributes = $this->atts['html_attr']; + $attributes = is_callable($this->atts['html_attr']) + ? (array) call_user_func($this->atts['html_attr']) + : (array) ($this->atts['html_attr'] ?? []); + unset($attributes['class']);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (36)
inc/functions/helper.php(3 hunks)inc/list-tables/class-base-list-table.php(8 hunks)inc/models/class-site.php(1 hunks)inc/ui/class-field.php(3 hunks)views/admin-pages/fields/field-group.php(2 hunks)views/admin-pages/fields/field-header.php(2 hunks)views/admin-pages/fields/field-html.php(2 hunks)views/admin-pages/fields/field-note.php(2 hunks)views/admin-pages/fields/field-small-header.php(2 hunks)views/admin-pages/fields/field-text-display.php(2 hunks)views/admin-pages/fields/field-text-edit.php(5 hunks)views/admin-pages/fields/field-toggle.php(3 hunks)views/admin-pages/fields/form.php(3 hunks)views/base/edit/widget-tabs.php(2 hunks)views/checkout/fields/field-checkbox.php(1 hunks)views/checkout/fields/field-html.php(2 hunks)views/checkout/fields/field-note.php(1 hunks)views/checkout/fields/field-select.php(3 hunks)views/checkout/fields/field-submit.php(1 hunks)views/checkout/fields/field-toggle.php(1 hunks)views/checkout/fields/form.php(2 hunks)views/checkout/templates/template-selection/clean.php(1 hunks)views/checkout/templates/template-selection/legacy.php(2 hunks)views/checkout/templates/template-selection/minimal.php(2 hunks)views/dashboard-statistics/widget-revenue.php(4 hunks)views/dashboard-widgets/current-membership-product-details.php(1 hunks)views/dashboard-widgets/thank-you.php(5 hunks)views/events/widget-initiator.php(2 hunks)views/events/widget-message.php(2 hunks)views/legacy/signup/pricing-table/plan.php(3 hunks)views/legacy/signup/signup-main.php(1 hunks)views/legacy/signup/signup-nav-links.php(1 hunks)views/settings/fields/field-text.php(2 hunks)views/shortcodes/shortcodes.php(2 hunks)views/wizards/host-integrations/activation.php(1 hunks)views/wizards/setup/default.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- views/legacy/signup/signup-main.php
🚧 Files skipped from review as they are similar to previous changes (7)
- views/admin-pages/fields/field-toggle.php
- views/checkout/fields/field-note.php
- views/checkout/fields/field-html.php
- views/admin-pages/fields/field-header.php
- views/admin-pages/fields/field-text-display.php
- views/checkout/fields/field-submit.php
- views/admin-pages/fields/field-text-edit.php
🧰 Additional context used
🧬 Code graph analysis (24)
views/dashboard-widgets/current-membership-product-details.php (1)
inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/events/widget-initiator.php (1)
inc/list-tables/class-base-list-table.php (2)
column_membership(849-893)column_payment(903-949)
views/wizards/setup/default.php (1)
inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/wizards/host-integrations/activation.php (2)
inc/models/class-site.php (1)
get_description(549-556)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/base/edit/widget-tabs.php (1)
inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/checkout/templates/template-selection/minimal.php (1)
inc/models/class-site.php (1)
get_preview_url_attrs(430-438)
views/dashboard-widgets/thank-you.php (1)
inc/functions/currency.php (1)
wu_format_currency(367-401)
views/admin-pages/fields/form.php (2)
inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)inc/ui/class-field.php (1)
print_html_attributes(442-469)
views/dashboard-statistics/widget-revenue.php (2)
inc/functions/markup-helpers.php (1)
wu_tooltip_text(144-147)inc/functions/currency.php (1)
wu_format_currency(367-401)
views/events/widget-message.php (2)
inc/models/class-event.php (1)
get_message(254-259)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/admin-pages/fields/field-small-header.php (2)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
inc/ui/class-field.php (2)
inc/ui/class-form.php (1)
print_html_attributes(210-226)inc/functions/markup-helpers.php (1)
wu_print_html_attributes(104-109)
views/admin-pages/fields/field-html.php (2)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/admin-pages/fields/field-group.php (2)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/checkout/templates/template-selection/legacy.php (1)
inc/models/class-site.php (2)
get_preview_url_attrs(430-438)get_active_site_url(1206-1219)
views/checkout/templates/template-selection/clean.php (1)
inc/models/class-site.php (2)
get_preview_url_attrs(430-438)get_active_site_url(1206-1219)
views/checkout/fields/form.php (2)
inc/ui/class-field.php (1)
print_html_attributes(442-469)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/settings/fields/field-text.php (3)
inc/ui/class-field.php (1)
print_html_attributes(442-469)inc/functions/settings.php (1)
wu_get_setting(39-42)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/checkout/fields/field-select.php (2)
inc/ui/class-field.php (2)
print_wrapper_html_attributes(477-484)print_html_attributes(442-469)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
views/checkout/fields/field-checkbox.php (3)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)inc/functions/markup-helpers.php (1)
wu_tooltip(120-135)
views/checkout/fields/field-toggle.php (3)
inc/ui/class-field.php (2)
print_wrapper_html_attributes(477-484)print_html_attributes(442-469)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)inc/functions/markup-helpers.php (1)
wu_tooltip(120-135)
views/legacy/signup/pricing-table/plan.php (3)
inc/models/class-base-model.php (1)
get_id(427-430)inc/functions/currency.php (2)
wu_get_currency_symbol(174-354)wu_format_currency(367-401)inc/traits/trait-wp-ultimo-plan-deprecated.php (1)
is_featured_plan(96-103)
views/admin-pages/fields/field-note.php (2)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/helper.php (1)
wu_kses_allowed_html(326-516)
inc/list-tables/class-base-list-table.php (4)
inc/functions/admin.php (1)
wu_render_empty_state(20-36)inc/models/class-site.php (1)
get_membership(997-1030)inc/models/class-payment.php (1)
get_membership(278-281)inc/list-tables/class-line-item-list-table.php (1)
get_payment(52-57)
🔇 Additional comments (47)
views/checkout/templates/template-selection/legacy.php (1)
27-28: Good: explicit type hint for $sites.This improves static analysis and IDE help.
views/checkout/templates/template-selection/minimal.php (2)
84-84: Good: inline type hint on loop variable.Helps tools and maintainers.
113-115: Ensure anchor gets href and rel=noopener via helper.With the proposed get_preview_url_attrs() change, this anchor will have a proper href, target, and rel, fixing keyboard/JS-off behavior.
After updating the helper, verify the rendered includes href, target, and rel.
inc/list-tables/class-base-list-table.php (2)
500-511: Correct: switch from echo to direct renderwu_render_empty_state() renders output; removing echo prevents accidental “1”/empty echoes. LGTM.
1169-1183: LGTM: safer JSON encoding and clearer formattingThe wp_json_encode usage is correct and improves readability.
views/events/widget-initiator.php (2)
25-34: LGTM: inline get_avatar() echo is fineDirect echo keeps things simple; args are already constrained.
100-105: These calls now print directly; ensure upstream null branches returnThis relies on column_membership()/column_payment() emitting output and returning on “not found”. column_payment() currently misses the return, which will fatal. Fix in inc/list-tables/class-base-list-table.php as suggested.
views/legacy/signup/pricing-table/plan.php (2)
105-105: Good switch to wp_kses_post for line items.Allows intended markup while remaining safe.
126-126: Button looks fine; ensure surrounding form handles CSRF.Confirm a nonce is present in the wrapping form that processes
plan_id.views/dashboard-widgets/thank-you.php (2)
97-97: Good hardening: currency outputs are now escaped.Wrapping wu_format_currency(...) with esc_html(...) is correct and consistent with wp.org expectations.
Also applies to: 205-205, 396-396, 409-409, 417-417, 426-426, 432-432
59-60: No changes needed for date parsing. get_date_created() returns a date string, so the strtotime() call is required to convert it to a Unix timestamp for date_i18n.views/dashboard-statistics/widget-revenue.php (1)
14-14: LGTM: direct tooltip emission + escaped currency.
- wu_tooltip_text(...) used as an emitter is correct here.
- All currency outputs are now escaped.
Also applies to: 19-19, 35-35, 51-51, 86-86
views/wizards/setup/default.php (1)
20-20: Verify merging logic and tighten allowlist
Confirm that wu_kses_allowed_html() actually merges your Vue/data attributes into each base tag (rather than using a top-level array “+” that won’t extend existing tags). If it doesn’t, switch to explicit per-tag merges (e.g.$allowed[$tag] = isset($allowed[$tag]) ? $allowed[$tag] + $attrs : $attrs;). Also consider using a stricter allowlist for untrusted content (e.g.wp_kses_post()) and reserve Vue directives for internal templates only.views/base/edit/widget-tabs.php (1)
7-7: ABSPATH guard style normalization: LGTMviews/wizards/host-integrations/activation.php (1)
15-15: Sanitizing integration description: goodConfirm that allowing SVG and data/Vue attributes here is intended. For content derived from integrations or remote sources, consider wp_kses_post() or a reduced allowlist.
views/events/widget-message.php (1)
7-7: ABSPATH guard style normalization: LGTMviews/checkout/fields/form.php (4)
7-7: LGTM — ABSPATH guard normalized.
13-13: Avoid duplicate form attributes from the printer.
If Form::print_html_attributes() may include method/id, this duplicates the explicitly-set attributes here.Would you confirm Form::print_html_attributes() unsets at least: method, id (and action if present)?
17-17: Guard $step usage to avoid notices.
$step may be undefined or not an object.- <<?php echo esc_attr($form->wrap_tag); ?> class="<?php echo esc_attr(trim($form->classes ? $form->classes . ' ' . $step->classes . ' wu-mt-2' : $step->classes . ' wu-mt-2')); ?>" <?php $form->print_html_attributes(); ?>> + <?php $step_classes = isset($step) && is_object($step) && isset($step->classes) ? $step->classes : ''; ?> + <<?php echo esc_attr($form->wrap_tag); ?> class="<?php echo esc_attr(trim(($form->classes ? $form->classes . ' ' : '') . $step_classes . ' wu-mt-2')); ?>" <?php $form->print_html_attributes(); ?>>
27-27: LGTM — fields output sanitized with wp_kses().views/admin-pages/fields/field-group.php (4)
7-7: LGTM — ABSPATH guard normalized.
9-9: LGTM — helpful docblock.
12-12: LGTM — wrapper attributes now printed (no echo needed).
57-57: LGTM — description sanitized with wp_kses().views/admin-pages/fields/form.php (5)
7-8: LGTM — guard + docblock.
13-13: LGTM — sanitized $form->before.
28-28: Ensure no duplicate id/class on.
When reusing Form::print_html_attributes() for the, confirm it unsets id and class before printing.
30-30: LGTM — sanitized rendered fields.
40-40: LGTM — sanitized $form->after.views/admin-pages/fields/field-html.php (3)
7-8: LGTM — guard + docblock.
11-11: LGTM — wrapper attrs printed (no echo/ignore needed).
45-45: LGTM — content sanitized via wp_kses().views/settings/fields/field-text.php (3)
7-7: LGTM — ABSPATH guard normalized.
29-29: LGTM — append sanitized.
37-37: LGTM — description sanitized.views/checkout/fields/field-checkbox.php (2)
7-8: Guard + local var annotation: OK.
Consistent with project style.
15-19: Escaping/sanitization: OK.
Title, tooltip, and description are safely handled.views/checkout/fields/field-toggle.php (3)
7-8: Guard + local var annotation: OK.
11-11: Wrapper attributes rely on Field::print_wrapper_html_attributes().
Ensure the duplicate-class fix in inc/ui/class-field.php is applied (see earlier comment).
15-21: Toggle markup and escaping: OK.
Direct attribute printing and kses usage look good.views/admin-pages/fields/field-note.php (3)
7-9: Guard + local var annotation: OK.
11-11: Wrapper attributes depend on Field::print_wrapper_html_attributes().
Apply the duplicate-class fix in inc/ui/class-field.php.
31-31: Sanitized description: OK.
Swapping to wp_kses(...) is the right move.views/checkout/fields/field-select.php (1)
11-11: print_ attribute usage: OK.*
Direct printing removes prior phpcs ignores and is consistent with Field API.Also applies to: 34-34
views/admin-pages/fields/field-small-header.php (3)
7-9: Guard + local var annotation: OK.
11-11: Wrapper attributes depend on Field::print_wrapper_html_attributes().
Apply the duplicate-class fix in inc/ui/class-field.php.
33-33: Sanitized description: OK.
Good swap to wp_kses(...).
| $vue_and_data_attributes = [ | ||
| // Vue.js directives | ||
| 'v-html' => true, | ||
| 'v-show' => true, | ||
| 'v-if' => true, | ||
| 'v-else' => true, | ||
| 'v-else-if' => true, | ||
| 'v-for' => true, | ||
| 'v-model' => true, | ||
| 'v-bind' => true, | ||
| 'v-on' => true, | ||
| 'v-cloak' => true, | ||
| 'v-pre' => true, | ||
| 'v-once' => true, | ||
| 'v-text' => true, | ||
| // Vue.js shorthand attributes | ||
| ':class' => true, | ||
| ':style' => true, | ||
| 'v-on:click' => true, | ||
| 'v-on:input' => true, | ||
| 'v-on:change' => true, | ||
| '@click' => true, | ||
| '@submit' => true, | ||
| '@change' => true, | ||
| // Common data attributes | ||
| 'data-image' => true, | ||
| 'data-src' => true, | ||
| 'data-id' => true, | ||
| 'data-value' => true, | ||
| 'data-target' => true, | ||
| 'data-toggle' => true, | ||
| 'data-dismiss' => true, | ||
| 'data-placement' => true, | ||
| 'data-content' => true, | ||
| 'data-title' => true, | ||
| 'data-delay' => true, | ||
| 'data-animation' => true, | ||
| 'data-container' => true, | ||
| 'data-trigger' => true, | ||
| // others | ||
| 'style' => true, | ||
| ]; |
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.
🛠️ Refactor suggestion
Critical: Allow-listing Vue directive/event attributes enables script execution; remove them.
Attributes like v-html, v-on, @click, :style evaluate JS. wp.org reviewers will flag this as XSS surface if used on user-provided content.
- $vue_and_data_attributes = [
- // Vue.js directives
- 'v-html' => true,
- 'v-show' => true,
- 'v-if' => true,
- 'v-else' => true,
- 'v-else-if' => true,
- 'v-for' => true,
- 'v-model' => true,
- 'v-bind' => true,
- 'v-on' => true,
- 'v-cloak' => true,
- 'v-pre' => true,
- 'v-once' => true,
- 'v-text' => true,
- // Vue.js shorthand attributes
- ':class' => true,
- ':style' => true,
- 'v-on:click' => true,
- 'v-on:input' => true,
- 'v-on:change' => true,
- '@click' => true,
- '@submit' => true,
- '@change' => true,
- // Common data attributes
- 'data-image' => true,
- 'data-src' => true,
- 'data-id' => true,
- 'data-value' => true,
- 'data-target' => true,
- 'data-toggle' => true,
- 'data-dismiss' => true,
- 'data-placement' => true,
- 'data-content' => true,
- 'data-title' => true,
- 'data-delay' => true,
- 'data-animation' => true,
- 'data-container' => true,
- 'data-trigger' => true,
- // others
- 'style' => true,
- ];
+ // Keep this minimal and safe for user-provided content.
+ $extra_common_attributes = [
+ 'style' => true,
+ 'data-*' => true,
+ // Allow v-cloak only (non-evaluated, used to hide until hydrated).
+ 'v-cloak' => true,
+ ];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In inc/functions/helper.php around lines 354 to 395, the current allow-list
includes Vue directives and event attributes (v-html, v-on, @click, :style,
etc.) which can execute JavaScript and create XSS risks; remove all Vue-specific
directives and event handlers from the allow-list and only keep non-executable
attributes (e.g., plain data-* attributes and safe presentation attributes like
style if absolutely required and sanitized). Update the array to exclude any
items that begin with "v-" or "@" or ":" and ensure backend
sanitization/escaping is applied to user-provided content instead of permitting
client-side JS hooks; add or update unit/integration tests to assert those
attributes are stripped and document the change in the code comment so reviewers
know Vue bindings are not allowed on user content.
| ?> | ||
| <div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | ||
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | ||
| <div class=''> | ||
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>{$not_found}</span> | ||
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'><?php esc_html_e('No membership found', 'multisite-ultimate'); ?></span> | ||
| </div> | ||
| </div>"; | ||
| <?php | ||
| return; |
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.
Stray characters break markup in “no membership” branch
There’s a literal "; after the closing div, which will output garbage and is a regression.
- </div>";
+ </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.
| ?> | |
| <div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | |
| <div class=''> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>{$not_found}</span> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'><?php esc_html_e('No membership found', 'multisite-ultimate'); ?></span> | |
| </div> | |
| </div>"; | |
| <?php | |
| return; | |
| ?> | |
| <div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | |
| <div class=''> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'><?php esc_html_e('No membership found', 'multisite-ultimate'); ?></span> | |
| </div> | |
| </div> | |
| <?php | |
| return; |
🤖 Prompt for AI Agents
In inc/list-tables/class-base-list-table.php around lines 854 to 862 there are
stray characters ("; ) after the closing div that will print garbage; remove the
extra literal characters so the HTML block is output correctly (either by
deleting the stray quote+semicolon or fixing the surrounding PHP echo/closing
tags) and ensure the PHP opening/closing tags and return remain valid so the "no
membership" branch renders without the spurious characters.
| printf( | ||
| "<a href='%s' class='wu-no-underline wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-pl-4 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | ||
| <div class='wu-absolute wu-top-0 wu-bottom-0 wu-left-0 wu-w-2 %s'> </div> | ||
| <div class=''> | ||
| <strong class='wu-block'>{$reference} <small class='wu-font-normal'>(#{$id})</small></strong> | ||
| <small>{$description}</small> | ||
| <strong class='wu-block'>%s <small class='wu-font-normal'>(#%s)</small></strong> | ||
| <small>%s</small> | ||
| </div> | ||
| </a>"; | ||
|
|
||
| return $html; | ||
| </a>", | ||
| esc_attr($membership_link), | ||
| esc_attr($status_classes), | ||
| esc_html($reference), | ||
| esc_html($id), | ||
| esc_html($description) | ||
| ); |
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.
🛠️ Refactor suggestion
Use esc_url() for href
Href values should use esc_url().
- esc_attr($membership_link),
+ esc_url($membership_link),📝 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.
| printf( | |
| "<a href='%s' class='wu-no-underline wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-pl-4 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <div class='wu-absolute wu-top-0 wu-bottom-0 wu-left-0 wu-w-2 %s'> </div> | |
| <div class=''> | |
| <strong class='wu-block'>{$reference} <small class='wu-font-normal'>(#{$id})</small></strong> | |
| <small>{$description}</small> | |
| <strong class='wu-block'>%s <small class='wu-font-normal'>(#%s)</small></strong> | |
| <small>%s</small> | |
| </div> | |
| </a>"; | |
| return $html; | |
| </a>", | |
| esc_attr($membership_link), | |
| esc_attr($status_classes), | |
| esc_html($reference), | |
| esc_html($id), | |
| esc_html($description) | |
| ); | |
| printf( | |
| "<a href='%s' class='wu-no-underline wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-pl-4 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <div class='wu-absolute wu-top-0 wu-bottom-0 wu-left-0 wu-w-2 %s'> </div> | |
| <div class=''> | |
| <strong class='wu-block'>%s <small class='wu-font-normal'>(#%s)</small></strong> | |
| <small>%s</small> | |
| </div> | |
| </a>", | |
| esc_url($membership_link), | |
| esc_attr($status_classes), | |
| esc_html($reference), | |
| esc_html($id), | |
| esc_html($description) | |
| ); |
🤖 Prompt for AI Agents
inc/list-tables/class-base-list-table.php around lines 879-892: the href is
currently escaped with esc_attr($membership_link) but URLs should be escaped
with esc_url(); update the printf call so the href value uses
esc_url($membership_link) (leave the other escaping calls as-is) ensuring the
URL is properly sanitized for output in the href attribute.
| public function column_payment($item): void { | ||
|
|
||
| $payment = $item->get_payment(); | ||
|
|
||
| if ( ! $payment) { | ||
| $not_found = __('No payment found', 'multisite-ultimate'); | ||
|
|
||
| return "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | ||
| printf( | ||
| "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | ||
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | ||
| <div class=''> | ||
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>{$not_found}</span> | ||
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> | ||
| </div> | ||
| </div>"; | ||
| </div>", | ||
| esc_html($not_found) | ||
| ); | ||
| } |
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.
Return after “No payment found” output to avoid fatal on null $payment
Without returning, subsequent $payment->... calls will fatal.
printf(
"<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'>
<span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span>
<div class=''>
<span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span>
</div>
</div>",
esc_html($not_found)
);
+ return;📝 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.
| public function column_payment($item): void { | |
| $payment = $item->get_payment(); | |
| if ( ! $payment) { | |
| $not_found = __('No payment found', 'multisite-ultimate'); | |
| return "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| printf( | |
| "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | |
| <div class=''> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>{$not_found}</span> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> | |
| </div> | |
| </div>"; | |
| </div>", | |
| esc_html($not_found) | |
| ); | |
| } | |
| public function column_payment($item): void { | |
| $payment = $item->get_payment(); | |
| if ( ! $payment) { | |
| $not_found = __('No payment found', 'multisite-ultimate'); | |
| printf( | |
| "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> | |
| <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> | |
| <div class=''> | |
| <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> | |
| </div> | |
| </div>", | |
| esc_html($not_found) | |
| ); | |
| return; | |
| } | |
| // ... existing logic that assumes $payment is non-null |
🤖 Prompt for AI Agents
In inc/list-tables/class-base-list-table.php around lines 903 to 919, the method
prints a "No payment found" card but doesn't return, so later code will call
methods on a null $payment and cause a fatal; after the printf that outputs the
"No payment found" markup, add an immediate return (or wrap the remaining logic
in an else) so execution stops when $payment is falsy.
| <?php if ($field->options_template) : ?> | ||
|
|
||
| <?php echo $field->options_template; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> | ||
| <?php echo wp_kses($field->options_template, wu_kses_allowed_html()); ?> |
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.
💡 Verification agent
🧩 Analysis chain
options_template will be stripped by kses (option/optgroup not allowed).
wu_kses_allowed_html() doesn’t include option/optgroup; the sanitized output will drop your dynamic options.
Option A (preferred): extend wu_kses_allowed_html().
In inc/functions/helper.php:
return $allowed_html + [
... SVG entries ...
-] + array_fill_keys(['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img'], $vue_and_data_attributes);
+] + array_fill_keys(['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img'], $vue_and_data_attributes)
+ + [
+ 'optgroup' => [
+ 'label' => true,
+ 'disabled' => true,
+ 'class' => true,
+ 'id' => true,
+ ],
+ 'option' => [
+ 'value' => true,
+ 'selected' => true,
+ 'disabled' => true,
+ 'label' => true,
+ 'class' => true,
+ 'id' => true,
+ // whitelist common data- attrs you actually use
+ 'data-id' => true,
+ 'data-value' => true,
+ 'data-src' => true,
+ ],
+ ];Option B (localize): in this template, restrict to a dedicated allowlist:
-<?php echo wp_kses($field->options_template, wu_kses_allowed_html()); ?>
+<?php echo wp_kses($field->options_template, [
+ 'optgroup' => ['label' => true, 'disabled' => true, 'class' => true, 'id' => true],
+ 'option' => ['value' => true, 'selected' => true, 'disabled' => true, 'label' => true, 'class' => true, 'id' => true, 'data-id' => true, 'data-value' => true, 'data-src' => true],
+]); ?>Also confirm multi-select support: if print_html_attributes() can output multiple, ensure name ends with [] when multiple is set.
🏁 Script executed:
#!/bin/bash
set -eo pipefail
# 1. Locate the wu_kses_allowed_html() function and show its context
rg -nP "function\s+wu_kses_allowed_html" -C5 inc/functions/helper.php
# 2. Check whether 'optgroup' or 'option' tags are already whitelisted
rg -nP "'optgroup'\s*=>" -n inc/functions/helper.php || echo "No optgroup entries found"
rg -nP "'option'\s*=>" -n inc/functions/helper.php || echo "No option entries found"
# 3. Locate the print_html_attributes() function to inspect name/multiple handling
rg -nP "function\s+print_html_attributes" -C5 .Length of output: 1924
🏁 Script executed:
#!/bin/bash
set -eo pipefail
# Show the full wu_kses_allowed_html function definition (around its start and return)
sed -n '320,380p' inc/functions/helper.php
# Show print_html_attributes in class-field.php to inspect name/multiple handling
sed -n '430,550p' inc/ui/class-field.php
# Search for handling of 'name' and 'multiple' attributes in class-field.php
rg -nP "'name'\s*=>" -C3 inc/ui/class-field.php || echo "No explicit name handling found"
rg -nP "'multiple'" -C3 inc/ui/class-field.php || echo "No explicit multiple handling found"Length of output: 3593
🏁 Script executed:
#!/bin/bash
set -eo pipefail
# Print the beginning of the select field template to inspect <select> attributes
if [[ -f views/checkout/fields/field-select.php ]]; then
sed -n '1,200p' views/checkout/fields/field-select.php
else
echo "views/checkout/fields/field-select.php not found"
fiLength of output: 1559
Extend allowed HTML to include <option>/<optgroup> and fix multi-select name
The default wu_kses_allowed_html() omits <option> and <optgroup>, so wp_kses($field->options_template, …) will strip your dynamic options. Update inc/functions/helper.php to whitelist those tags with their attributes (value, label, selected, disabled, etc.), or inline a dedicated allowlist in the template.
Also, when rendering a <select multiple>, append [] to the name (e.g. name="<?= esc_attr($field->id) ?>[]") so PHP receives all selected values.
| <?php $is_template ? $site_template->get_preview_url_attrs() : printf('href="%s" target="_blank"', esc_attr($site_template->get_active_site_url())); ?> | ||
| > |
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.
🛠️ Refactor suggestion
Fix URL escaping, add rel for target="_blank", and avoid side-effect ternary
- Use esc_url() for href values.
- Add rel="noopener noreferrer" when using target="_blank".
- Prefer an explicit if/else over a side-effect-only ternary for readability.
Apply:
- <?php $is_template ? $site_template->get_preview_url_attrs() : printf('href="%s" target="_blank"', esc_attr($site_template->get_active_site_url())); ?>
+ <?php if ($is_template) { $site_template->get_preview_url_attrs(); } else { printf('href="%s" target="_blank" rel="noopener noreferrer"', esc_url($site_template->get_active_site_url())); } ?>Optional (in inc/models/class-site.php) — drop inline JS onclick for accessibility and wp.org review friendliness:
public function get_preview_url_attrs(): void {
if ( ! Template_Previewer::get_instance()->get_setting('enabled', true)) {
printf('href="%s" target="_blank" rel="noopener noreferrer"', esc_url($this->get_active_site_url()));
return;
}
printf('href="%s" target="_blank" rel="noopener noreferrer"', esc_url(add_query_arg('open', 1, $this->get_preview_url())));
}🤖 Prompt for AI Agents
In views/checkout/templates/template-selection/clean.php around lines 114-115,
replace the side-effect-only ternary that prints href attrs with an explicit
if/else that outputs a properly escaped href using esc_url() and includes
rel="noopener noreferrer" whenever target="_blank" is used (avoid printing via
printf inside a ternary). Also consider updating inc/models/class-site.php
get_preview_url_attrs() to output href using esc_url(add_query_arg(...)) or
esc_url($this->get_active_site_url()) and include target="_blank" rel="noopener
noreferrer", and remove any inline onclick JS for accessibility/wp.org
guidelines.
|
|
||
| <span class="wu-my-1 wu-inline-block"> | ||
| <?php echo $object->get_message(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> | ||
| <?php wp_kses($object->get_message(), wu_kses_allowed_html()); ?> |
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.
Message not rendered — missing echo
wp_kses() returns the sanitized string; without echo, nothing is printed.
- <?php wp_kses($object->get_message(), wu_kses_allowed_html()); ?>
+ <?php echo wp_kses($object->get_message(), wu_kses_allowed_html()); ?>📝 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.
| <?php wp_kses($object->get_message(), wu_kses_allowed_html()); ?> | |
| <?php echo wp_kses($object->get_message(), wu_kses_allowed_html()); ?> |
🤖 Prompt for AI Agents
In views/events/widget-message.php at line 16, the call to
wp_kses($object->get_message(), wu_kses_allowed_html()) returns a sanitized
string but is not echoed, so nothing is output; fix it by echoing the result
(i.e., output the return value of wp_kses) so the sanitized message is printed
to the page.
| <div id="plan-<?php echo esc_attr($plan->get_id()); ?>" data-plan="<?php echo esc_attr($plan->get_id()); ?>" | ||
| <?php | ||
|
|
||
| foreach ([1, 3, 12] as $type) { | ||
| $price = $plan->free ? __('Free!', 'multisite-ultimate') : str_replace(wu_get_currency_symbol(), '', wu_format_currency((((float) $plan->{'price_' . $type}) / $type))); | ||
| $plan_attrs .= " data-price-$type='$price'"; | ||
| $price = $plan->free ? __('Free!', 'multisite-ultimate') : str_replace(wu_get_currency_symbol(), '', wu_format_currency((((float) $plan->{'price_' . $type}) / $type))); | ||
| printf(" data-price-%s='%s'", esc_attr($type), esc_attr($price)); | ||
| } | ||
|
|
||
| $plan_attrs = apply_filters('wu_pricing_table_plan', $plan_attrs, $plan); | ||
|
|
||
| ?> |
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.
💡 Verification agent
🧩 Analysis chain
Make data-price values numeric and stable; avoid localized strings/formatting and direct property access.
Formatted strings (and “Free!”) in data-* break JS parsing and comparisons; also use is_free() for consistency. Consider BC check for the removed wu_pricing_table_plan hook.
-foreach ([1, 3, 12] as $type) {
- $price = $plan->free ? __('Free!', 'multisite-ultimate') : str_replace(wu_get_currency_symbol(), '', wu_format_currency((((float) $plan->{'price_' . $type}) / $type)));
- printf(" data-price-%s='%s'", esc_attr($type), esc_attr($price));
-}
+foreach ([1, 3, 12] as $type) {
+ $raw = $plan->is_free() ? 0 : (float) $plan->{"price_$type"};
+ // If the consumer expects monthly values instead of per-period totals, use:
+ // $raw = $plan->is_free() ? 0 : ((float) $plan->{"price_$type"}) / $type;
+ printf(' data-price-%s="%s"', esc_attr($type), esc_attr(number_format($raw, 2, '.', '')));
+}Run to confirm expectations and usages:
🏁 Script executed:
#!/bin/bash
# JS consumers of data-price-*, and any usages of removed/changed hooks.
rg -nP -C3 --type=js '(data-price-(1|3|12))|dataset\.price' || true
rg -n "wu_pricing_table_plan|wu_plan_select_button_attributes" -C2 || trueLength of output: 767
🏁 Script executed:
#!/bin/bash
# Search for is_free method definition in PHP files
rg -nP --type=php '\bfunction\s+is_free\s*\(' -C2 || true
# Search for usage of is_free() method
rg -nP --type=php '->is_free\s*\(' -C2 || true
# Search for direct access to $plan->free property
rg -nP --type=php '\$plan->free\b' -C2 || true
# Search for class declarations with "Plan" in the name
rg -nP --type=php 'class\s+\w*Plan\b' -C2 || trueLength of output: 2729
Use $plan->is_free() and emit unformatted numeric data-price-* values
Replace the localized string logic (__('Free!')/wu_format_currency) and direct $plan->free access with:
$raw = $plan->is_free() ? 0 : (float) $plan->{"price_$type"};
printf(
' data-price-%s="%s"',
esc_attr($type),
esc_attr(number_format($raw, 2, '.', ''))
);This produces stable numeric values for JS (no consumers of data-price-* found) and avoids formatting issues. The BC note for wu_pricing_table_plan can be removed—no such hook exists.
🤖 Prompt for AI Agents
In views/legacy/signup/pricing-table/plan.php around lines 25 to 33, replace the
direct $plan->free check and localized/formatted price output with a numeric,
unformatted data-price-* value using $plan->is_free() to determine zero or the
raw float price for each billing period; emit data-price-%s attributes with
esc_attr(number_format($raw, 2, '.', '')) (or equivalent) so JS receives stable
numeric strings, and remove any localized "Free!" logic and unnecessary BC
note/hook mention.
|
|
||
| <?php $i = 1; foreach ($nav_links as $link => $label) : ?> | ||
|
|
||
| <a href="<?php echo esc_attr($link); ?>"> |
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.
Use esc_url() for href to prevent javascript: and malformed schemes from filters
$nav_links is filterable; esc_attr() won’t strip dangerous schemes. esc_url() is the correct sanitization for href/src in WP.
- <a href="<?php echo esc_attr($link); ?>">
+ <a href="<?php echo esc_url($link); ?>">📝 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.
| <a href="<?php echo esc_attr($link); ?>"> | |
| <a href="<?php echo esc_url($link); ?>"> |
🤖 Prompt for AI Agents
In views/legacy/signup/signup-nav-links.php around line 51, the anchor href is
currently sanitized with esc_attr(), which does not block dangerous URL schemes;
replace esc_attr($link) with esc_url($link) so the href is properly sanitized
for URL contexts (ensuring javascript: and malformed schemes are stripped) and
preserve existing escaping pattern.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/functions/helper.php (1)
507-513: Security: wp_kses does not protocol-check xlink:href — fix required.Verified: in WP 6.6.x wp_kses applies protocol filtering for "href" but does not include the namespaced "xlink:href" in its URI-attribute checks, so javascript:/data: URIs can bypass filtering.
Action (pick one):
- Add "xlink:href" to wp_kses_uri_attributes via the wp_kses_uri_attributes filter, e.g.
add_filter('wp_kses_uri_attributes', fn($attrs)=>array_merge($attrs, ['xlink:href']));- Or strip/validate xlink:href from uploaded SVGs before calling wp_kses.
Location: inc/functions/helper.php lines 507–513 (also 515–522).
♻️ Duplicate comments (2)
inc/functions/helper.php (2)
354-406: Remove Vue directives and event bindings from KSES allow‑list (wp.org will flag as XSS).v-html, v-on, @click, :style, etc. can execute JS when hydrated and must not be permitted on user-provided content. Keep only non-executable attributes (e.g., data-* you control, tabindex, class/id, optionally v-cloak). Gate any framework bindings behind trusted/admin-only rendering, not KSES.
Apply this minimal, safe replacement:
- $vue_and_data_attributes = [ - // Vue.js directives - 'v-html' => true, - 'v-show' => true, - 'v-if' => true, - 'v-else' => true, - 'v-else-if' => true, - 'v-for' => true, - 'v-model' => true, - 'v-bind' => true, - 'v-bind:class' => true, - 'v-on' => true, - 'v-cloak' => true, - 'v-pre' => true, - 'v-once' => true, - 'v-text' => true, - // Vue.js shorthand attributes - ':class' => true, - ':style' => true, - 'v-on:click' => true, - 'v-on:input' => true, - 'v-on:change' => true, - '@click' => true, - '@submit' => true, - '@change' => true, - // Common data attributes - 'data-image' => true, - 'data-src' => true, - 'data-id' => true, - 'data-value' => true, - 'data-target' => true, - 'data-toggle' => true, - 'data-dismiss' => true, - 'data-placement' => true, - 'data-content' => true, - 'data-title' => true, - 'data-delay' => true, - 'data-animation' => true, - 'data-container' => true, - 'data-trigger' => true, - 'data-model' => true, - 'data-value-field' => true, - 'data-label-field' => true, - 'data-search-field' => true, - 'data-max-items' => true, - 'data-init' => true, - 'tabindex' => true, - // others - 'style' => true, - 'class' => true, - 'id' => true, - ]; + $extra_common_attributes = [ + // Safe, non-executable attributes for user content + 'tabindex' => true, + 'class' => true, + 'id' => true, + // Allow 'style' only if you rely on WP's safecss_filter_attr downstream. + 'style' => true, + // Enumerate only the data-* you actually need + 'data-image' => true, + 'data-src' => true, + 'data-id' => true, + 'data-value' => true, + 'data-target' => true, + 'data-toggle' => true, + 'data-dismiss' => true, + 'data-placement' => true, + 'data-content' => true, + 'data-title' => true, + 'data-delay' => true, + 'data-animation' => true, + 'data-container' => true, + 'data-trigger' => true, + 'data-model' => true, + 'data-value-field' => true, + 'data-label-field' => true, + 'data-search-field' => true, + 'data-max-items' => true, + 'data-init' => true, + // Non-executable framework helper allowed + 'v-cloak' => true, + ];
556-556: Fix merge logic: array_merge_recursive introduces invalid nested values; array + with array_keys drops additions.
- array_merge_recursive creates arrays for duplicate attribute keys (e.g., 'class' => [true, true]), which breaks kses expectations.
- array_keys($allowed_html) + ['div', ...] uses array union by numeric keys; right-hand values are discarded. Net effect: your explicit tag list is ignored.
Refactor to per-tag union and return after extending:
- ] + array_merge_recursive($allowed_html, array_fill_keys(array_keys($allowed_html) + ['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input', 'textarea'], $vue_and_data_attributes)); + ]; + + // Extend common tags with extra safe attributes without nesting. + $tags_to_extend = array_unique(array_merge( + array_keys($allowed_html), + ['div','span','p','h1','h2','h3','h4','h5','h6','strong','em','b','i','ul','ol','li','a','img','input','textarea'] + )); + foreach ($tags_to_extend as $tag) { + $allowed_html[$tag] = isset($allowed_html[$tag]) + ? ($allowed_html[$tag] + $extra_common_attributes) // key-union keeps existing values; adds new ones + : $extra_common_attributes; + } + + $svg_allow = [ + 'svg' => $svg_attributes + [ + 'width' => true, + 'height' => true, + 'viewbox' => true, + 'preserveaspectratio' => true, + 'version' => true, + 'baseprofile' => true, + ], + // ... keep the rest of the SVG map exactly as-is ... + ]; + + return $svg_allow + $allowed_html;
🧹 Nitpick comments (2)
inc/functions/helper.php (2)
441-448: Add canonical SVG attribute casing (current list only has lowercase variants).Common generators emit viewBox/preserveAspectRatio/baseProfile. Include both forms for compatibility.
'svg' => $svg_attributes + [ - 'width' => true, - 'height' => true, - 'viewbox' => true, - 'preserveaspectratio' => true, - 'version' => true, - 'baseprofile' => true, + 'width' => true, + 'height' => true, + 'viewbox' => true, + 'viewBox' => true, + 'preserveaspectratio' => true, + 'preserveAspectRatio' => true, + 'version' => true, + 'baseprofile' => true, + 'baseProfile' => true, ], @@ 'image' => $svg_attributes + [ 'href' => true, 'xlink:href' => true, 'x' => true, 'y' => true, 'width' => true, 'height' => true, - 'preserveaspectratio' => true, + 'preserveaspectratio' => true, + 'preserveAspectRatio' => true, ],Also applies to: 514-522
328-352: Optional: expand a11y attributes on SVG.aria-label and tabindex are commonly needed alongside aria-labelledby/aria-describedby; consider adding them to $svg_attributes for better accessibility.
'aria-hidden' => true, + 'aria-label' => true, 'aria-labelledby' => true, 'aria-describedby' => true, 'role' => true, 'focusable' => true,
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
inc/ui/class-base-element.php (3)
646-653: Shortcode callback must return markup (now it prints); wrap with output bufferingdisplay() now prints and returns void. add_shortcode callbacks are expected to return a string; printing here will render out-of-band and can break content placement. Wrap display() in output buffering for shortcodes.
Apply:
- add_shortcode($this->get_shortcode_id(), [$this, 'display']); + // Shortcodes must return markup; capture printed output from display(). + add_shortcode($this->get_shortcode_id(), function ($atts, $content = null) { + ob_start(); + $this->display($atts); + return (string) ob_get_clean(); + });
1017-1041: display_template() now receives null; capture printed outputSince display() prints, $content becomes null and str_replace() on null risks deprecation notices on PHP 8.1+. Capture output first.
Apply:
- public function display_template($atts) { - - $content = $this->display($atts); + public function display_template($atts) { + // Capture printed output from display(). + ob_start(); + $this->display($atts); + $content = (string) ob_get_clean();
1246-1255: is_preview() ignores computed value; preview will never triggerMethod computes $is_preview but returns a filter defaulting to false, so ensure_setup() will always choose setup(). Use the computed value as the filter’s base.
Apply:
- return apply_filters('wu_element_is_preview', false, $this); + return apply_filters('wu_element_is_preview', $is_preview, $this);inc/admin-pages/class-customer-edit-admin-page.php (1)
1006-1006: Escape else-path output.Return value should be escaped to avoid unsafe HTML if country names ever change upstream.
- $html = $country_name; + $html = esc_html($country_name);
♻️ Duplicate comments (5)
inc/list-tables/class-base-list-table.php (5)
835-843: Escape aria-label and dynamic datetime textEscape the aria-label and the human‑readable parts while keeping
/ markup.- $text = $formatted_value . sprintf('<br><small>%s</small>', sprintf($placeholder, human_time_diff($time))); - - return sprintf('<span role="tooltip" aria-label="%s">%s</span>', date_i18n('Y-m-d H:i:s', $time), $text); + $relative = sprintf($placeholder, human_time_diff($time)); + $text = sprintf('%s<br><small>%s</small>', esc_html($formatted_value), esc_html($relative)); + return sprintf('<span role="tooltip" aria-label="%s">%s</span>', esc_attr( date_i18n('Y-m-d H:i:s', $time) ), $text);
858-866: Remove stray '";' after closing divThis literal breaks markup in the “no membership” branch.
- </div>"; + </div>
883-896: Use esc_url() for href in membership linkHref should be URL‑escaped, not attribute‑escaped.
- esc_attr($membership_link), + esc_url($membership_link),
914-923: Return after printing “No payment found” to avoid fatalSubsequent $payment->... calls will otherwise fatal on null.
printf( "<div class='wu-table-card wu-text-gray-700 wu-py-1 wu-px-2 wu-flex wu-flex-grow wu-block wu-rounded wu-items-center wu-border wu-border-solid wu-border-gray-300 wu-relative wu-overflow-hidden'> <span class='dashicons dashicons-wu-block wu-text-gray-600 wu-px-1 wu-pr-3'> </span> <div class=''> <span class='wu-block wu-py-3 wu-text-gray-600 wu-text-2xs wu-font-bold wu-uppercase'>%s</span> </div> </div>", esc_html($not_found) ); + return;
939-952: Use esc_url() for href in payment linkSame as membership: sanitize URL with esc_url().
- esc_attr($payment_link), + esc_url($payment_link),
🧹 Nitpick comments (7)
inc/ui/class-billing-info-element.php (2)
265-273: Stale docblock vs. void outputDocblock still says “@return string” though output() is now void and directly renders. Update it.
Apply:
- * @return string + * @return void Outputs HTML directly.
271-293: Silence PHPMD unused $content; retain void-output contractAdd an explicit unset to avoid PHPMD noise.
Apply:
- public function output($atts, $content = null): void { + public function output($atts, $content = null): void { + // Signature enforced by Base_Element; not used here. + unset($content);inc/ui/class-current-site-element.php (1)
351-354: Update docblock and silence unused $contentDoc now says void—good. Add unset to quiet PHPMD; also adjust the descriptive text in the docblock to “Outputs HTML directly.”
Apply:
- * @return void + * @return void Outputs HTML directly. */ - public function output($atts, $content = null): void { + public function output($atts, $content = null): void { + unset($content);inc/ui/class-limits-element.php (1)
237-241: Make doc reflect void and silence unused $contentSame pattern: update doc text and unset $content.
Apply:
- * @return void + * @return void Outputs HTML directly. */ - public function output($atts, $content = null): void { + public function output($atts, $content = null): void { + unset($content);inc/ui/class-account-summary-element.php (1)
291-295: Doc now void — add unset($content) and clarify doc textSmall tidy-up.
Apply:
- * @return void + * @return void Outputs HTML directly. */ - public function output($atts, $content = null): void { + public function output($atts, $content = null): void { + unset($content);inc/ui/class-invoices-element.php (2)
271-271: Docblock now mismatches behavior; update wording and suppress PHPMD warningThe text still says “Should return HTML markup…”, but the method is now side‑effecting. Also, PHPMD flags
$contentas unused.Apply this docblock tweak:
@@ - * Should return HTML markup to be used to display the block. - * This method is shared between the block render method and - * the shortcode implementation. + * Outputs HTML directly (side‑effects). This method is shared between + * the block render method and the shortcode implementation. Callers + * must buffer output if a string is required. @@ - * @return void + * @return void + * @SuppressWarnings(PHPMD.UnusedFormalParameter)Alternative (if you prefer not to suppress): add this as the first line of the method body:
public function output($atts, $content = null): void { + // keep signature in sync with Base_Element, silence unused param + unset($content);Also applies to: 261-266
277-280: Early return on missing membership is fine; consider clarifying intentGiven setup() already calls set_display(false), this guard is a safe double‑check. Add a brief note that output can still be invoked in preview/SEO contexts where setup() might have short‑circuited.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
composer.json(1 hunks)inc/admin-pages/class-customer-edit-admin-page.php(1 hunks)inc/checkout/class-checkout.php(1 hunks)inc/class-wp-ultimo.php(1 hunks)inc/integrations/host-providers/class-cloudflare-host-provider.php(1 hunks)inc/list-tables/class-base-list-table.php(6 hunks)inc/list-tables/class-customer-list-table.php(1 hunks)inc/models/class-site.php(1 hunks)inc/ui/class-account-summary-element.php(2 hunks)inc/ui/class-base-element.php(6 hunks)inc/ui/class-billing-info-element.php(3 hunks)inc/ui/class-current-site-element.php(2 hunks)inc/ui/class-invoices-element.php(1 hunks)inc/ui/class-limits-element.php(2 hunks)readme.txt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- inc/checkout/class-checkout.php
- inc/integrations/host-providers/class-cloudflare-host-provider.php
- inc/class-wp-ultimo.php
- readme.txt
- inc/list-tables/class-customer-list-table.php
- composer.json
- inc/models/class-site.php
🧰 Additional context used
🧬 Code graph analysis (7)
inc/ui/class-account-summary-element.php (3)
inc/ui/class-base-element.php (2)
output(210-210)ensure_setup(451-456)inc/ui/class-billing-info-element.php (1)
output(271-293)inc/functions/template.php (1)
wu_get_template(20-73)
inc/ui/class-base-element.php (5)
inc/admin-pages/class-dashboard-admin-page.php (1)
output(571-583)inc/admin-pages/class-edit-admin-page.php (1)
output(770-783)inc/deprecated/deprecated.php (1)
output(220-220)inc/ui/class-account-summary-element.php (1)
output(293-311)inc/functions/markup-helpers.php (1)
wu_tooltip(120-135)
inc/list-tables/class-base-list-table.php (5)
inc/functions/admin.php (1)
wu_render_empty_state(20-36)inc/models/class-site.php (1)
get_membership(997-1030)inc/models/class-payment.php (1)
get_membership(278-281)inc/checkout/class-cart.php (2)
get_membership(2617-2620)get_payment(2628-2631)inc/list-tables/class-line-item-list-table.php (1)
get_payment(52-57)
inc/ui/class-billing-info-element.php (4)
inc/ui/class-account-summary-element.php (1)
output(293-311)inc/ui/class-base-element.php (2)
output(210-210)ensure_setup(451-456)inc/ui/class-invoices-element.php (1)
output(273-285)inc/functions/template.php (1)
wu_get_template(20-73)
inc/ui/class-current-site-element.php (3)
inc/ui/class-account-summary-element.php (1)
output(293-311)inc/ui/class-base-element.php (2)
output(210-210)ensure_setup(451-456)inc/functions/template.php (1)
wu_get_template(20-73)
inc/ui/class-invoices-element.php (2)
inc/ui/class-base-element.php (2)
output(210-210)ensure_setup(451-456)inc/functions/template.php (1)
wu_get_template(20-73)
inc/ui/class-limits-element.php (3)
inc/ui/class-account-summary-element.php (1)
output(293-311)inc/ui/class-base-element.php (2)
output(210-210)ensure_setup(451-456)inc/functions/template.php (1)
wu_get_template(20-73)
🪛 PHPMD (2.15.0)
inc/ui/class-account-summary-element.php
293-293: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
inc/ui/class-billing-info-element.php
271-271: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
inc/ui/class-current-site-element.php
353-353: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
inc/ui/class-invoices-element.php
273-273: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
inc/ui/class-limits-element.php
239-239: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (15)
inc/list-tables/class-base-list-table.php (1)
504-516: Empty-state render path looks correctDirectly calling wu_render_empty_state() (no echo) matches the template printer pattern.
inc/ui/class-billing-info-element.php (1)
51-61: Docblock type updates look goodSwitch to concrete model types for $membership and $site improves IDE/helpfulness.
inc/ui/class-base-element.php (5)
208-211: Abstract output(): void — aligned across elementsInterface change is clear and consistent with the new direct-render pattern.
451-456: ensure_setup() is a good guardPrevents output-before-setup bugs and double-setup. Nice.
1111-1112: Inline widget render path updated correctlySwitched from echoing a returned string to calling display() that prints. Matches the new contract.
1192-1193: Metabox render path updated correctlySame as inline path—now calls display() which prints. Good.
1233-1233: Direct wu_tooltip() call is consistent with printing helpersGood move to standardize on direct printers.
inc/ui/class-current-site-element.php (2)
358-360: Null-site guard is correctEarly return avoids rendering during SEO/static passes without a site.
401-401: Switch to wu_get_template() is correct for direct outputAligned with the new rendering approach.
inc/ui/class-limits-element.php (2)
243-246: Null-site guard is correctPrevents rendering without context.
276-276: wu_get_template() switch looks goodDirect rendering fits the new Base_Element contract.
inc/ui/class-account-summary-element.php (2)
297-300: Null-site guard is correctMatches the pattern used across elements.
310-310: Direct template render aligns with new patternLooks good.
inc/ui/class-invoices-element.php (2)
284-285: Switch to wu_get_template(): confirm the invoices view emits outputSince we’re including the view directly, ensure
views/dashboard-widgets/invoices.phpechoes content (no reliance on returns). If any filters expect a string, buffer around this call.If needed, wrap with buffering at call sites (example pattern):
- wu_get_template('dashboard-widgets/invoices', $atts); + // Caller-side pattern when a string is required: + // ob_start(); + wu_get_template('dashboard-widgets/invoices', $atts); + // $html = ob_get_clean(); + // return $html;
273-274: Contract change to void — confirm no callers expect a returned stringSearches returned no matches; cannot confirm. Verify that no code does
return $el->output(...),echo $el->output(...), or assigns from->output(...), and ensure anyadd_shortcode/register_block_typerender callbacks buffer output and return the buffered string. Location: inc/ui/class-invoices-element.php (lines 273–274). Run locally: rg -nP '(?:return|echo)\s+[^\n]->output\s(' ; rg -nP '$\w+\s*=\s*[^\n]->output\s(' ; rg -nP '\badd_shortcode\s*(|\bregister_block_type\s*(' ; rg -nP '\bwu_get_template_contents\s*('
| '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>', | ||
| $country_name, | ||
| wu_tooltip_text($country_name), | ||
| esc_attr($country_name), | ||
| wu_get_flag_emoji((string) $country_code) |
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.
💡 Verification agent
🧩 Analysis chain
Fix ARIA misuse and escape output in render_country().
role="tooltip" on a static span is incorrect and causes duplicate announcements; the flag is decorative because the country name is already printed. Also escape outputs per WP standards.
- '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>',
- $country_name,
- esc_attr($country_name),
- wu_get_flag_emoji((string) $country_code)
+ '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" aria-hidden="true">%s</span>',
+ esc_html($country_name),
+ esc_html(wu_get_flag_emoji((string) $country_code))Optional (if you truly need a tooltip later): use a trigger with aria-describedby pointing to a hidden tooltip element, not role="tooltip" on the icon itself.
To check for similar cases:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'role\s*=\s*"tooltip"' --type=phpLength of output: 8035
Fix ARIA misuse and escape outputs across the codebase
role="tooltip" is applied to static/decorative elements (causes duplicate screen-reader announcements). Replace decorative icons with aria-hidden="true", escape outputs with esc_html/esc_attr, and implement real tooltips using a trigger + aria-describedby when required. Prefer fixing the helper(s) in inc/functions/markup-helpers.php to avoid repetition.
Occurrences to update (from your search):
- views/ui/jumper-trigger.php:11
- views/ui/container-toggle.php:11
- views/events/widget-initiator.php:38,64
- views/dashboard-statistics/widget-countries.php:68
- views/dashboard-widgets/activity-stream.php:49
- inc/admin-pages/class-customer-edit-admin-page.php:1000
- inc/list-tables/class-customer-list-table.php:147
- inc/list-tables/class-discount-code-list-table.php:173
- inc/list-tables/class-membership-list-table.php:218
- inc/list-tables/class-base-list-table.php:842
- inc/integrations/host-providers/class-cloudflare-host-provider.php:119-122
- inc/functions/markup-helpers.php:126,146
Apply the local fix (example) for inc/admin-pages/class-customer-edit-admin-page.php:
- '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>',
- $country_name,
- esc_attr($country_name),
- wu_get_flag_emoji((string) $country_code)
+ '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" aria-hidden="true">%s</span>',
+ esc_html($country_name),
+ esc_html(wu_get_flag_emoji((string) $country_code))Confirm before committing with:
rg -nP -C2 'role\s*=\s*"tooltip"' --type=php
📝 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.
| '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" role="tooltip" aria-label="%s">%s</span>', | |
| $country_name, | |
| wu_tooltip_text($country_name), | |
| esc_attr($country_name), | |
| wu_get_flag_emoji((string) $country_code) | |
| '<span>%s</span><span class="wu-flag-icon wu-w-5 wu-ml-1" aria-hidden="true">%s</span>', | |
| esc_html($country_name), | |
| esc_html(wu_get_flag_emoji((string) $country_code)) |
🤖 Prompt for AI Agents
In inc/admin-pages/class-customer-edit-admin-page.php around lines 1000 to 1003,
replace the decorative flag span that currently uses role="tooltip" with a span
that is aria-hidden="true" and ensure the displayed country name and attributes
are properly escaped (use esc_html for visible text and esc_attr for
attributes); update or add a helper in inc/functions/markup-helpers.php to
render flag icons with aria-hidden and proper escaping to avoid repeating fixes
across files, and if a real tooltip is required implement a separate trigger
element plus aria-describedby pointing to a visually-hidden tooltip element.
After making changes run: rg -nP -C2 'role\s*=\s*"tooltip"' --type=php to
confirm no remaining incorrect usages before committing.
This will stay open until we finally get approval for inclusion in wp.org repo. It will have some things remove from the main branch, like translations which can't be included in the package.
Summary by CodeRabbit