Open
Conversation
WalkthroughThe change updates modules/system/controllers/eventlogs/_field_details.php: inside the Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
modules/system/controllers/eventlogs/_field_details.php (1)
5-5: ⚡ Quick winHardcoded
DETAILSlabel bypasses i18n
<h4>DETAILS</h4>is a raw English string. Every other visible label in this file usese(trans('system::lang.event_log.details.*')). This label should go through the same pipeline, both for consistency and to support localization.♻️ Proposed fix
- '<h4>DETAILS</h4><pre style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>', + '<h4>%s</h4><pre style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>', + e(trans('system::lang.event_log.details.details')),Add the corresponding key to the language file(s):
// modules/system/lang/en/lang.php (and other locales) 'details' => 'Details',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/system/controllers/eventlogs/_field_details.php` at line 5, Replace the hardcoded "<h4>DETAILS</h4>" with a translatable call consistent with the rest of the file (e.g. use e(trans('system::lang.event_log.details.title')) or the existing event_log.details namespace) and add the corresponding key/value ("Details") to the language files (modules/system/lang/en/lang.php and other locales) so the label flows through the i18n pipeline like the other labels in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/system/controllers/eventlogs/_field_details.php`:
- Around line 3-8: The DETAILS block currently renders print_r($value, true)
raw, causing stored XSS; change it to escape the rendered string before output
(e.g., wrap the print_r result with the template's escaping helper such as e()
or htmlspecialchars) in the printf call so the output is safely HTML-encoded;
update the printf invocation that prints the DETAILS preformatted block (the
block using printf and print_r) to pass an escaped string instead of the raw
print_r output.
---
Nitpick comments:
In `@modules/system/controllers/eventlogs/_field_details.php`:
- Line 5: Replace the hardcoded "<h4>DETAILS</h4>" with a translatable call
consistent with the rest of the file (e.g. use
e(trans('system::lang.event_log.details.title')) or the existing
event_log.details namespace) and add the corresponding key/value ("Details") to
the language files (modules/system/lang/en/lang.php and other locales) so the
label flows through the i18n pipeline like the other labels in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e811d3d9-bac4-4f04-afad-666a60c82ec0
📒 Files selected for processing (1)
modules/system/controllers/eventlogs/_field_details.php
jaxwilko
commented
May 6, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
modules/system/controllers/eventlogs/_field_details.php (1)
4-7: 💤 Low valueInline styles on
<pre>diverge from the existing<style>block convention.The rest of the template centralises all presentation in the
<style>block at line 105. Moving the new<pre>styles there keeps the stylesheet consistent and makes future theming changes easier.♻️ Proposed refactor
Add to the
<style>block (around line 105):+ `#winter-log-viewer` pre.details-dump { + background: `#fff`; + padding: 10px; + border: 1px solid `#ddd`; + }Update the
printfcall:- '<h4>DETAILS</h4><pre style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>', + '<h4>DETAILS</h4><pre class="details-dump">%s</pre>', e(print_r($value, true))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/system/controllers/eventlogs/_field_details.php` around lines 4 - 7, The inline styles on the <pre> element should be moved into the existing stylesheet and the HTML updated to use a class; add a new CSS rule in the template's <style> block (matching the existing convention) for a selector like .eventlog-details pre (or .details-pre) to include background, padding and border, then modify the printf call that prints e(print_r($value, true)) to output <pre class="...">%s</pre> instead of the inline style so presentation is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/system/controllers/eventlogs/_field_details.php`:
- Line 5: The template contains a hardcoded heading string '<h4>DETAILS</h4><pre
style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>' which
bypasses i18n; replace the '<h4>DETAILS</h4>' fragment with a translated call
(use e(trans(...)) consistent with the rest of the file) referencing a new
language key such as 'event_log.details' and add the corresponding 'details' =>
'Details' entry under the event_log (or module-specific) array in the language
files so non-English backends receive the translated heading.
---
Nitpick comments:
In `@modules/system/controllers/eventlogs/_field_details.php`:
- Around line 4-7: The inline styles on the <pre> element should be moved into
the existing stylesheet and the HTML updated to use a class; add a new CSS rule
in the template's <style> block (matching the existing convention) for a
selector like .eventlog-details pre (or .details-pre) to include background,
padding and border, then modify the printf call that prints e(print_r($value,
true)) to output <pre class="...">%s</pre> instead of the inline style so
presentation is centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87ac5ed3-5961-4840-b831-8b1397433018
📒 Files selected for processing (1)
modules/system/controllers/eventlogs/_field_details.php
| if (!isset($value['logVersion']) || $value['logVersion'] !== 2) { | ||
| if ($value ?? false) { | ||
| printf( | ||
| '<h4>DETAILS</h4><pre style="background: #fff; padding: 10px; border: 1px solid #ddd;">%s</pre>', |
There was a problem hiding this comment.
Hardcoded DETAILS label bypasses i18n.
Every other user-visible string in this template goes through trans()/e(trans(...)). The new <h4>DETAILS</h4> heading is the only hardcoded English label, which will break non-English backends.
🌐 Proposed fix
- '<h4>DETAILS</h4><pre style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>',
+ '<h4>%s</h4><pre style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>',
+ e(trans('system::lang.event_log.details.details')),
e(print_r($value, true))Add the corresponding translation key to the language files, e.g.:
// modules/system/lang/en/lang.php (under event_log.details)
'details' => 'Details',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/system/controllers/eventlogs/_field_details.php` at line 5, The
template contains a hardcoded heading string '<h4>DETAILS</h4><pre
style="background: `#fff`; padding: 10px; border: 1px solid `#ddd`;">%s</pre>' which
bypasses i18n; replace the '<h4>DETAILS</h4>' fragment with a translated call
(use e(trans(...)) consistent with the rest of the file) referencing a new
language key such as 'event_log.details' and add the corresponding 'details' =>
'Details' entry under the event_log (or module-specific) array in the language
files so non-English backends receive the translated heading.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a very basic viewer for event log details.
Should resolve #1475.
I'd like to add better support for the new log viewer, but don't have time currently. To anybody reading this you can use:
To force in custom logs in the new style with fancy stack traces, but use at your own risk because it's fairly easy to break the renderer while doing this.
Summary by CodeRabbit