-
Notifications
You must be signed in to change notification settings - Fork 39
Fix WPML Compatibility Issue with Event Bubbles Revert #631
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds WPML language propagation to AJAX requests and localized script variables, updates shortcode name in documentation, and wraps qTip2 initialization in try-catch. JavaScript now uses shared ajaxData for grid/list navigation and includes lang when available. PHP appends lang to admin-ajax URL and exposes wpml_language to scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant UI as Calendar UI (JS)
participant WP as WordPress admin-ajax.php
participant WPML as WPML (lang context)
Note over UI: Localized vars include wpml_language (if defined)
User->>UI: Navigate calendar (grid/list)
UI->>UI: Build ajaxData { action, ... , id, lang? }
alt WPML active
UI->>WP: AJAX request with lang query + ajaxData.lang
WP->>WPML: Resolve language context
WPML-->>WP: Language applied
else No WPML
UI->>WP: AJAX request without lang
end
WP-->>UI: JSON response (events/calendar HTML)
rect rgba(230,250,230,0.6)
Note right of UI: Initialize qTip2 tooltips
UI->>UI: try { qTip2(...) } catch { warn and continue }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 (1)
assets/js/default-calendar.js (1)
114-124: Rename variable to avoid redeclaration.The variable
ajaxDatais declared again here, which was flagged by static analysis. While both declarations are in separate scopes (grid vs. list navigation), using different variable names would improve code clarity.Based on learnings
Apply this diff to use distinct variable names:- var ajaxData = { + var listAjaxData = { action: action, ts: timestamp, id: id, }; // WPML compatibility: Include language parameter if available if (simcal_default_calendar.wpml_language) { - ajaxData.lang = simcal_default_calendar.wpml_language; + listAjaxData.lang = simcal_default_calendar.wpml_language; } $.ajax({ url: simcal_default_calendar.ajax_url, type: 'POST', dataType: 'json', cache: false, - data: ajaxData, + data: listAjaxData,Alternatively, rename the grid navigation variable to
gridAjaxDatafor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CLAUDE.md(1 hunks)assets/js/default-calendar.js(3 hunks)includes/functions/shared.php(1 hunks)includes/main.php(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
assets/js/default-calendar.js
[error] 114-114: Shouldn't redeclare 'ajaxData'. Consider to delete it or rename it.
'ajaxData' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (4)
CLAUDE.md (1)
16-16: LGTM! Documentation updated to reflect correct shortcode.The shortcode reference has been updated from
[calendar]to[simple_calendar], which aligns with the plugin's naming convention.assets/js/default-calendar.js (2)
65-76: LGTM! AJAX data properly structured with WPML support.The grid navigation now uses a shared
ajaxDataobject and correctly includes the language parameter when WPML is active.
353-408: LGTM! Robust error handling for qTip2 initialization.The try-catch block gracefully handles qTip2 initialization failures, preventing calendar rendering from breaking. The error is logged for debugging while allowing the calendar to continue functioning.
includes/main.php (1)
277-284: Confirm server-side handling oflangin AJAX handlers.The
langquery parameter is appended client-side but neitherdraw_list_ajaxnordraw_grid_ajaxreads or validates it. If WPML language support is intended:
- Read
$_REQUEST['lang']in each handler- Sanitize with
sanitize_text_field()- Validate against your language pattern (e.g.
/^[a-z]{2}(?:_[A-Z]{2})?$/)- Pass the sanitized
$langinto your drawing methodsOtherwise, remove the client-side append to avoid unused parameters.
| // WPML compatibility: Add current language code to JavaScript variables | ||
| if (defined('ICL_LANGUAGE_CODE')) { | ||
| $vars['wpml_language'] = ICL_LANGUAGE_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.
🧩 Analysis chain
Verify that ICL_LANGUAGE_CODE is safe for JavaScript output.
The WPML language code is being added to JavaScript variables without explicit sanitization. While ICL_LANGUAGE_CODE is a WPML-provided constant, ensure it's safe for direct output to JavaScript.
Run the following script to verify how WPML defines and uses this constant:
Additionally, consider applying esc_js() or similar sanitization if the constant could contain untrusted data:
// WPML compatibility: Add current language code to JavaScript variables
if (defined('ICL_LANGUAGE_CODE')) {
- $vars['wpml_language'] = ICL_LANGUAGE_CODE;
+ $vars['wpml_language'] = esc_js(ICL_LANGUAGE_CODE);
}🏁 Script executed:
#!/bin/bash
# Description: Check WPML's ICL_LANGUAGE_CODE usage and validation
# Search for ICL_LANGUAGE_CODE definition and usage patterns in WPML contexts
rg -nP --type=php -C5 "define\s*\(\s*['\"]ICL_LANGUAGE_CODE['\"]"
# Look for any sanitization patterns around language code usage
rg -nP --type=php -C3 "ICL_LANGUAGE_CODE.*esc_|sanitize_"Length of output: 30646
Sanitize WPML language code before JS export
Wrap ICL_LANGUAGE_CODE in esc_js() to ensure safe JavaScript output:
// WPML compatibility: Add current language code to JavaScript variables
if (defined('ICL_LANGUAGE_CODE')) {
- $vars['wpml_language'] = ICL_LANGUAGE_CODE;
+ $vars['wpml_language'] = esc_js(ICL_LANGUAGE_CODE);
}📝 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.
| // WPML compatibility: Add current language code to JavaScript variables | |
| if (defined('ICL_LANGUAGE_CODE')) { | |
| $vars['wpml_language'] = ICL_LANGUAGE_CODE; | |
| } | |
| // WPML compatibility: Add current language code to JavaScript variables | |
| if (defined('ICL_LANGUAGE_CODE')) { | |
| $vars['wpml_language'] = esc_js(ICL_LANGUAGE_CODE); | |
| } |
🤖 Prompt for AI Agents
In includes/functions/shared.php around lines 180 to 184, the WPML language
constant ICL_LANGUAGE_CODE is being exported directly into the $vars array for
JavaScript; wrap the value in esc_js() before assigning to
$vars['wpml_language'] so the language code is properly escaped for safe JS
output (e.g., $vars['wpml_language'] = esc_js(ICL_LANGUAGE_CODE)); ensure
esc_js() is available in this context or use a PHP fallback/explicit casting to
string before escaping.
|
This pull request has been marked as stale due to inactivity. It will be closed in 30 days if no further updates are made. |
Summary by CodeRabbit