Feat/shared dhtml toolbar - #151
Merged
Merged
Conversation
Clicking Size, Font, Colour or B/I/U/S with no text selected did nothing at all. The no-selection branch called setVisible() on an element with the id held in $_hiddenText (default xoopsHiddenText), but no renderer has ever emitted such an element, so xoopsGetElementById() returned null and the handler threw a TypeError before it could act. For xoopsSetElementAttribute() that branch was dead twice over: besides the missing element, it eval'd a setElementSize/Font/Color() helper and no setElement* function exists in the file. It is removed, so with no selection the tag pair is simply inserted at the caret, which is what clicking the button asks for. xoopsMakeStyle() keeps its branch for any custom renderer that does emit the element, but only takes it when the element and the handler both genuinely exist, and otherwise falls through to inserting the pair. Both eval() calls are gone with it. The remaining dispatch is an explicit map of the four style helpers, which is all the callers ever pass. xoopsGetSelect() returns null on browsers without a selection API, so the selection is normalised to an empty string before use: without that, removing the old length guard would have inserted the literal text "null" between the tags. xoopsCodeText() is a leftover from an older toolbar layout that had a separate "add text" box. Nothing in core or any bundled extension calls it and no renderer emits the elements it reads, so every path through it dereferenced null. It is kept, because it is a global a third-party module could still call, but now returns harmlessly instead of throwing.
The same editor showed a different toolbar in the control panel than on the
front end, and different again between front-end themes. The cause was not
CSS: renderFormDhtmlTAXoopsCode() and renderFormDhtmlTATypography() were
independently hand-written HTML in five renderers, and no admin theme selects
a renderer, so the control panel silently fell back to the Legacy one whose
Bootstrap class names no admin theme defines.
XoopsDhtmlToolbar now produces the toolbar once and all five renderers
delegate to it. Only renderFormDhtmlTextArea() is on XoopsFormRendererInterface;
the other two are protected helpers, so they are kept as thin delegates for any
third-party subclass that calls or overrides them. Each renderer still supplies
its own chrome -- the textarea, the preview fieldset and the script loader.
The markup is framework-neutral with its own xo-edtb-* classes and a
self-contained stylesheet, because the admin themes load no CSS framework at
all; every colour, radius and size is a custom property so a theme can restyle
it without touching markup. Dropdowns are native <details>, grouped per
textarea with name= so opening one closes the others, with a small script for
older engines plus click-outside and Escape handling. The typography row is
emitted server-side rather than written by document.write, and the colour list
is a curated palette in a class constant instead of 216 entries generated by a
nested loop. role="toolbar" and role="group" are now present, which only the
Bootstrap 5 renderer had.
Four contracts are preserved deliberately. The codeicon preload event still
fires with $code by reference at the same point, so modules can keep appending
buttons. TextSanitizer extensions keep their [$html, $js] return shape, and the
three divergent rewrites of their hardcoded btn-default classes become one.
The global JS function names are untouched, since extensions call them by name.
XoopsFormDhtmlTextArea's public surface is unchanged.
The stylesheet also restores resize:vertical on the editor's textarea: the
default and transition admin themes ship a universal `* { resize:none }` reset
which stripped the grip, so the control panel's editor could not be resized
while the front end's could.
A test asserts the substantive property: all five renderers emit an identical
toolbar for the same element.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Contributor
Reviewer's GuideIntroduce a shared, framework-neutral XoopsDhtmlToolbar for the DHTML textarea editor and refactor all form renderers and JS helpers to use it consistently, with supporting CSS/JS assets and unit tests, plus a small test bootstrap fix for locale registry and constants. Sequence diagram for rendering DHTML textarea with shared toolbarsequenceDiagram
participant Controller
participant Renderer as XoopsFormRenderer*
participant Toolbar as XoopsDhtmlToolbar
participant Element as XoopsFormDhtmlTextArea
Controller->>Renderer: renderFormDhtmlTextArea(Element)
Renderer->>Toolbar: new XoopsDhtmlToolbar()
Renderer->>Toolbar: render(Element)
activate Toolbar
Toolbar->>Toolbar: injectStylesheet()
Toolbar->>Toolbar: renderCodeButtons(Element)
Toolbar->>Toolbar: renderTypography(Element)
Toolbar->>Toolbar: renderCheckLength(Element)
Toolbar-->>Renderer: toolbarHtml
deactivate Toolbar
Renderer->>Renderer: build <textarea> markup
Renderer-->>Controller: toolbarHtml + textareaHtml
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #151 +/- ##
============================================
+ Coverage 19.29% 20.12% +0.83%
+ Complexity 8227 8200 -27
============================================
Files 672 673 +1
Lines 44266 43875 -391
============================================
+ Hits 8539 8832 +293
+ Misses 35727 35043 -684 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In XoopsDhtmlToolbar::renderCodeButtons() the imagemanager and smilies URLs now use $element->getName() directly without rawurlencode(), whereas the Tailwind renderer previously encoded the target ID; consider restoring URL encoding to avoid issues when textarea IDs contain characters that are not URL-safe.
- Each renderer currently creates a new XoopsDhtmlToolbar instance on every renderFormDhtmlTextArea() call; if these toolbars become heavier over time, you may want to share a single instance (e.g. via a static accessor) to avoid repeated construction and keep renderer wiring lean.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In XoopsDhtmlToolbar::renderCodeButtons() the imagemanager and smilies URLs now use $element->getName() directly without rawurlencode(), whereas the Tailwind renderer previously encoded the target ID; consider restoring URL encoding to avoid issues when textarea IDs contain characters that are not URL-safe.
- Each renderer currently creates a new XoopsDhtmlToolbar instance on every renderFormDhtmlTextArea() call; if these toolbars become heavier over time, you may want to share a single instance (e.g. via a static accessor) to avoid repeated construction and keep renderer wiring lean.
## Individual Comments
### Comment 1
<location path="htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php" line_range="153-154" />
<code_context>
+ $textareaId = $element->getName();
+ $btn = self::BTN_SM;
+
+ $code = '<a name="moresmiley"></a>';
+ $code .= '<div class="' . self::GROUP_CLASS . '" role="group" aria-label="' . _XOOPS_FORM_ALT_IMG . '">';
+ $code .= $this->button($btn, $this->jsCall('xoopsCodeUrl', [$textareaId, _ENTERURL, _ENTERWEBTITLE]), _XOOPS_FORM_ALT_URL, 'fa-solid fa-link');
+ $code .= $this->button($btn, $this->jsCall('xoopsCodeEmail', [$textareaId, _ENTEREMAIL, _ENTERWEBTITLE]), _XOOPS_FORM_ALT_EMAIL, 'fa-solid fa-envelope');
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The aria-label for the code button group is tied to the image alt text, which is misleading for screen readers.
The group’s aria-label `_XOOPS_FORM_ALT_IMG` doesn’t match its contents (URL, email, image, smilies, extension, code, quote), so screen readers treat the whole group as image-related. Please use a more generic label (e.g. a new constant like `_XOOPS_FORM_ARIA_CODEBAR`) or remove the group label and rely on the individual button labels instead.
Suggested implementation:
```
$code = '<a name="moresmiley"></a>';
$code .= '<div class="' . self::GROUP_CLASS . '" role="group">';
```
If you prefer a descriptive group label instead of relying solely on individual button labels, you can:
1. Define a new language constant (e.g. `_XOOPS_FORM_ARIA_CODEBAR`) in the appropriate language file.
2. Replace the edited line with:
`$code .= '<div class="' . self::GROUP_CLASS . '" role="group" aria-label="' . _XOOPS_FORM_ARIA_CODEBAR . '">';`
This keeps the toolbar accessible while making the label semantically accurate.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+153
to
+154
| $code = '<a name="moresmiley"></a>'; | ||
| $code .= '<div class="' . self::GROUP_CLASS . '" role="group" aria-label="' . _XOOPS_FORM_ALT_IMG . '">'; |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The aria-label for the code button group is tied to the image alt text, which is misleading for screen readers.
The group’s aria-label _XOOPS_FORM_ALT_IMG doesn’t match its contents (URL, email, image, smilies, extension, code, quote), so screen readers treat the whole group as image-related. Please use a more generic label (e.g. a new constant like _XOOPS_FORM_ARIA_CODEBAR) or remove the group label and rely on the individual button labels instead.
Suggested implementation:
$code = '<a name="moresmiley"></a>';
$code .= '<div class="' . self::GROUP_CLASS . '" role="group">';
If you prefer a descriptive group label instead of relying solely on individual button labels, you can:
- Define a new language constant (e.g.
_XOOPS_FORM_ARIA_CODEBAR) in the appropriate language file. - Replace the edited line with:
$code .= '<div class="' . self::GROUP_CLASS . '" role="group" aria-label="' . _XOOPS_FORM_ARIA_CODEBAR . '">';
This keeps the toolbar accessible while making the label semantically accurate.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, framework-neutral DHTML editor toolbar (XoopsDhtmlToolbar) and updates all XOOPS form renderers to delegate toolbar markup generation to it, while also modernizing legacy JavaScript helpers and expanding unit test coverage around toolbar behavior and extension integration.
Changes:
- Add
XoopsDhtmlToolbaras the single source of truth for DHTML toolbar markup (buttons, typography dropdowns, check-length) and shared assets (CSS/JS). - Refactor Legacy/Bootstrap3/4/5/Tailwind renderers to delegate toolbar construction to the shared class (keeping protected helpers for compatibility).
- Improve
formdhtmltextarea.jshelpers to avoid null dereferences and removeeval(), plus add tests and test bootstrap constants/cache cleanup.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarTest.php | Adds unit coverage validating renderer parity, preload event behavior, extension integration, and stylesheet injection. |
| tests/bootstrap.php | Defines missing editor-related language constants for tests and purges LocaleRegistry cache for deterministic runs. |
| htdocs/include/formdhtmltextarea.js | Hardens legacy editor JS helpers (null-safety, removes eval-based dispatch, improves no-selection behavior). |
| htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php | Delegates DHTML textarea toolbar rendering to the shared toolbar. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererLegacy.php | Delegates DHTML textarea toolbar rendering to the shared toolbar. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.php | Delegates DHTML textarea toolbar rendering to the shared toolbar. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap4.php | Delegates DHTML textarea toolbar rendering to the shared toolbar. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap3.php | Delegates DHTML textarea toolbar rendering to the shared toolbar. |
| htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php | New shared toolbar implementation (markup generation, extension button class rewrite, stylesheet/script injection). |
| htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.js | Adds framework-neutral <details> dropdown behavior (mutual exclusivity, click-outside, Escape handling). |
| htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.css | Adds framework-neutral toolbar styling and targeted CSS fixes for admin-theme resets. |
Suppressed comments (2)
htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php:208
renderTypography()also uses$element->getName()when generating JS handlers. SincegetName()HTML-escapes quotes, the string passed intoxoopsSetElementAttribute()can diverge from the real textarea id. Use the raw name viagetName(false).
public function renderTypography(XoopsFormDhtmlTextArea $element): string
{
$textareaId = $element->getName();
$hiddenText = (string) $element->_hiddenText;
htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php:253
renderCheckLength()passes$element->getName()into the JS call builder. As with the other toolbar rows, this can pass an HTML-escaped id into JavaScript and fail to find the textarea. Prefer the raw name (getName(false)) for JS arguments.
$maxlength = (int) $element->configs['maxlength'];
}
$onclick = $this->jsCall('XoopsCheckLength', [$element->getName(), (string) $maxlength, _XOOPS_FORM_ALT_LENGTH, _XOOPS_FORM_ALT_LENGTH_MAX]);
Comment on lines
+72
to
+78
| private const RENDERER_CLASSES = [ | ||
| 'XoopsFormRendererLegacy', | ||
| 'XoopsFormRendererBootstrap3', | ||
| 'XoopsFormRendererBootstrap4', | ||
| 'XoopsFormRendererBootstrap5', | ||
| 'XoopsFormRendererTailwind', | ||
| ]; |
Comment on lines
+357
to
+369
| protected function jsCall(string $fn, array $args): string | ||
| { | ||
| $parts = []; | ||
| foreach ($args as $arg) { | ||
| if (is_int($arg) || is_float($arg)) { | ||
| $parts[] = (string) $arg; | ||
| } else { | ||
| $parts[] = '"' . htmlspecialchars((string) $arg, ENT_QUOTES | ENT_HTML5, 'UTF-8') . '"'; | ||
| } | ||
| } | ||
|
|
||
| return $fn . '(' . implode(', ', $parts) . ');'; | ||
| } |
Comment on lines
+150
to
+159
| $textareaId = $element->getName(); | ||
| $btn = self::BTN_SM; | ||
|
|
||
| $code = '<a name="moresmiley"></a>'; | ||
| $code .= '<div class="' . self::GROUP_CLASS . '" role="group" aria-label="' . _XOOPS_FORM_ALT_IMG . '">'; | ||
| $code .= $this->button($btn, $this->jsCall('xoopsCodeUrl', [$textareaId, _ENTERURL, _ENTERWEBTITLE]), _XOOPS_FORM_ALT_URL, 'fa-solid fa-link'); | ||
| $code .= $this->button($btn, $this->jsCall('xoopsCodeEmail', [$textareaId, _ENTEREMAIL, _ENTERWEBTITLE]), _XOOPS_FORM_ALT_EMAIL, 'fa-solid fa-envelope'); | ||
| $code .= $this->button($btn, $this->jsCall('xoopsCodeImg', [$textareaId, _ENTERIMGURL, _ENTERIMGPOS, _IMGPOSRORL, _ERRORIMGPOS, _XOOPS_FORM_ALT_ENTERWIDTH]), _XOOPS_FORM_ALT_IMG, 'fa-solid fa-file-image'); | ||
| $code .= $this->button($btn, $this->jsCall('openWithSelfMain', [XOOPS_URL . '/imagemanager.php?target=' . $textareaId, 'imgmanager', 400, 430]), _XOOPS_FORM_ALT_IMAGE, 'fa-solid fa-file-image', '<span style="font-size:75%;"> Manager</span>'); | ||
| $code .= $this->button($btn, $this->jsCall('openWithSelfMain', [XOOPS_URL . '/misc.php?action=showpopups&type=smilies&target=' . $textareaId, 'smilies', 300, 475]), _XOOPS_FORM_ALT_SMILEY, 'fa-solid fa-face-smile'); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php (1)
485-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused Tailwind toolbar helper.
renderEditorButton(),BTN_NEUTRAL_SM, andDROPDOWN_MENU_CLSare not called insideXoopsFormRendererTailwind.php; the xoopscode and typography renderers delegate toXoopsDhtmlToolbar. Remove this deadprotectedhelper plus the two private constants, or mark it deprecated if intentional for downstream subclasses.🤖 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 `@htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php` around lines 485 - 495, Remove the unused renderEditorButton() helper from XoopsFormRendererTailwind along with the private constants BTN_NEUTRAL_SM and DROPDOWN_MENU_CLS. Do not alter the xoopscode or typography renderer delegation through XoopsDhtmlToolbar.
🤖 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 `@htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.css`:
- Around line 19-37: Move the default custom-property declarations from only
`.xo-edtb-toolbar` to `:root`, while retaining the `.xo-edtb-toolbar` block for
toolbar-specific overrides. Ensure standalone `.xo-edtb-group` rows returned by
`renderFormDhtmlTAXoopsCode()` and `renderFormDhtmlTATypography()` resolve the
button sizing, colors, and border variables without a toolbar ancestor.
- Around line 81-91: Add explicit :focus-visible styles for both .xo-edtb-btn
and .xo-edtb-menu-item, providing a clear visible focus indicator that remains
effective despite the admin theme reset. Keep the existing hover, active, and
sizing rules unchanged.
In `@htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.js`:
- Around line 58-70: Update the Escape key handler in the document keydown
listener to restore focus only for the open dropdown that currently contains the
active element. Continue closing all open dropdowns, but avoid moving focus when
typing outside a dropdown and ensure only the focused dropdown’s summary
receives focus.
In `@htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php`:
- Around line 212-230: Escape every language-derived aria-label in
XoopsDhtmlToolbar, including the existing call sites near the toolbar rendering
logic and the three group labels shown here, using a small shared esc helper
that applies htmlspecialchars with ENT_QUOTES | ENT_HTML5 and UTF-8. Update only
these aria-label interpolations; keep button() and dropdown() handling
unchanged.
- Around line 209-215: In the toolbar-building method, validate and normalize
both global overrides before use: accept only arrays for formtextdhtml_sizes and
formtextdhtml_fonts, otherwise use safe defaults, and ensure an empty sizes
array does not render an unusable Size dropdown. Keep font handling compatible
with array_combine by passing only a valid non-empty font list, using the
existing default-font symbol where needed.
In `@htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap3.php`:
- Around line 309-313: Choose one consistent contract for
renderFormDhtmlTATypography() and apply it to all five renderers:
htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap3.php#L309-L313,
htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap4.php#L314-L318,
htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.php#L315-L319,
htdocs/class/xoopsform/renderer/XoopsFormRendererLegacy.php#L224-L228, and
htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php#L524-L528. Either
remove renderCheckLength() and provide a separate renderFormDhtmlTACheckLength()
helper, or retain the combined output and update every method docblock to
document both controls; keep the Tailwind change aligned with the dead-helper
cleanup at Line 485.
In `@tests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarTest.php`:
- Around line 290-299: Make
testStylesheetIsInjectedOnceAcrossMultipleEditorsOnOnePage deterministic by
explicitly controlling the $GLOBALS['xoTheme'] state, preferably clearing it in
setUp() or at the start of the test. Preserve the existing assertions for
first-render injection and second-render suppression.
- Around line 262-265: Update testCheckLengthButtonCarriesConfiguredMaxlength
and the XoopsFormDhtmlTextArea configuration path to avoid dynamically assigning
configs; declare a typed public array $configs initialized to [] on the
appropriate class, or reuse an existing configuration accessor, while preserving
the test’s configured maxlength value.
---
Outside diff comments:
In `@htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php`:
- Around line 485-495: Remove the unused renderEditorButton() helper from
XoopsFormRendererTailwind along with the private constants BTN_NEUTRAL_SM and
DROPDOWN_MENU_CLS. Do not alter the xoopscode or typography renderer delegation
through XoopsDhtmlToolbar.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fb047ae1-dd84-44c6-bfb8-b2284a65f624
📒 Files selected for processing (11)
htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.phphtdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.csshtdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.jshtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap3.phphtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap4.phphtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.phphtdocs/class/xoopsform/renderer/XoopsFormRendererLegacy.phphtdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.phphtdocs/include/formdhtmltextarea.jstests/bootstrap.phptests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarTest.php
Renderers wrote element values straight into quoted attributes and textarea bodies. All five now route values through a shared XoopsFormRendererValueEscapeTrait. Toolbar and TextSanitizer extension handlers build their JavaScript arguments with json_encode instead of htmlspecialchars, which is the correct encoder for a JS string literal inside an HTML attribute. The extension-authoring sample in readme.txt is updated to match.
Three copies of core renderers had accumulated under the TinyMCE image-manager plugins and were installed as the global renderer, so those screens did not receive core renderer changes. They were stale snapshots with no local modifications; removed, and the endpoints now xoops_load() the core class. Separately, the image category create, update and delete handlers validated the CSRF token but not the caller's permission - the admin check applied only to which controls were rendered. Guards added to all five handlers.
…rer uniqueness Pinned methods keep their documented text-context exclusion but are now exercised with payloads that cannot express themselves in element text, so their attribute sites stay covered. Adds a per-element attribute-name-set assertion, per-extension coverage that bypasses the config gate, an onclick delimiter assertion, and a test that each renderer class is declared exactly once.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php:196
- The containment assertions for element/script/img counts are inverted: as written they pass when the hostile render has more nodes than the baseline (i.e., the injection case) and fail when it has fewer. This makes the test ineffective at catching breakouts that add elements/scripts/images.
self::assertLessThanOrEqual($baseline['elements'], $shape['elements'], "An element was injected by $where");
self::assertLessThanOrEqual($baseline['scripts'], $shape['scripts'], "A <script> was injected by $where");
self::assertLessThanOrEqual($baseline['images'], $shape['images'], "An <img> was injected by $where");
Greptile SummaryThe PR consolidates DHTML editor toolbar generation into a shared, framework-neutral implementation and updates all supported form renderers to delegate to it.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php | Introduces the shared toolbar implementation, context-aware escaping, extension integration, and one-time asset registration. |
| htdocs/include/formdhtmltextarea.js | Replaces eval-based style dispatch and hardens no-selection and missing-element behavior. |
| htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.js | Adds delegated dropdown exclusivity, outside-click handling, and Escape-key behavior. |
| htdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.css | Adds framework-neutral toolbar and dropdown styling with compatibility rules for existing themes. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererLegacy.php | Delegates legacy DHTML toolbar rendering to the shared implementation while retaining compatibility helpers. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.php | Delegates Bootstrap 5 DHTML toolbar generation to the shared implementation. |
| htdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.php | Delegates Tailwind DHTML toolbar generation to the shared implementation. |
| htdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/xoopsimagemanager.php | Loads the canonical core Bootstrap renderer instead of a plugin-local snapshot. |
| tests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarTest.php | Covers shared output parity, delegation, preload integration, length handling, and asset injection. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
F[XoopsFormDhtmlTextArea] --> R[Selected form renderer]
R --> T[XoopsDhtmlToolbar]
T --> C[Code and extension buttons]
T --> Y[Typography controls]
T --> L[Length check]
T --> A[Shared CSS and JavaScript assets]
Reviews (4): Last reviewed commit: "fix(textsanitizer): degrade invalid UTF-..." | Re-trigger Greptile
Address review findings on the shared dhtml toolbar: - Validate the shape of the formtextdhtml_fonts and formtextdhtml_sizes globals before use. Both are documented override points set by module code; a non-array value previously reached array_combine() (TypeError, killing the whole form render) or the dropdown foreach. Non-scalar font entries are dropped and an empty list falls back to the defaults. - Escape the concatenated language constants that land inside the four group aria-label attributes through a shared esc() helper, matching the escaping button() and dropdown() already apply to their titles. - Declare the configs property on XoopsFormDhtmlTextArea. Callers set it directly for the check-length maxlength, which is deprecated dynamic-property creation on PHP 8.2+ when undeclared. - toolbar.js: on Escape, still close every open dropdown but return focus only to the toggle of the dropdown that contained the active element, so Escape no longer pulls focus out of the textarea. - Document that renderFormDhtmlTATypography() returns the typography groups plus the check-length button in all five renderers - the same combined output the method produced before the toolbar extraction. - Pin $GLOBALS['xoTheme'] absent in the toolbar test setUp() (restored in tearDown()) so the stylesheet-injection assertions do not depend on test execution order.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php:196
- The containment assertions are reversed: as written they only fail when the hostile render has fewer nodes than the benign baseline, and they will not fail when an injection adds elements/scripts/images. The assertions should ensure the hostile shape does not exceed the baseline counts.
self::assertLessThanOrEqual($baseline['elements'], $shape['elements'], "An element was injected by $where");
self::assertLessThanOrEqual($baseline['scripts'], $shape['scripts'], "A <script> was injected by $where");
self::assertLessThanOrEqual($baseline['images'], $shape['images'], "An <img> was injected by $where");
tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php:398
- libxml_use_internal_errors(true) changes a global setting and isn't restored, which can leak into other tests and make later libxml warnings/errors harder to diagnose. Capture the previous value and restore it before returning.
libxml_use_internal_errors(true);
$doc->loadHTML('<!doctype html><meta charset="utf-8"><body>' . $html . '</body>', LIBXML_NOERROR);
libxml_clear_errors();
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php (1)
407-415: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle JSON encoding failures without aborting form rendering.
json_encode()withJSON_THROW_ON_ERRORthrowsJsonExceptionfor malformed UTF-8 in string arguments such as toolbar labels. Catch\Throwablenear the encoding step, emitE_USER_WARNING, and append a safe empty string so toolbar rendering can continue.🤖 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 `@htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php` around lines 407 - 415, Update jsCall’s argument-encoding branch to catch \Throwable from json_encode, emit an E_USER_WARNING, and append a safe empty-string argument when encoding fails. Preserve numeric handling and successful JSON encoding so malformed toolbar labels do not abort form rendering.Source: Coding guidelines
🤖 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 `@htdocs/class/textsanitizer/mms/mms.php`:
- Around line 39-45: Escape each localized title before concatenating it into
the HTML title attribute: in htdocs/class/textsanitizer/mms/mms.php lines 39-45,
apply htmlspecialchars with ENT_QUOTES | ENT_SUBSTITUTE and UTF-8 to
_XOOPS_FORM_ALTMMS; make the same change for _XOOPS_FORM_ALTMP3 in
htdocs/class/textsanitizer/mp3/mp3.php lines 34-36, _XOOPS_FORM_ALTRTSP in
htdocs/class/textsanitizer/rtsp/rtsp.php lines 39-45, _XOOPS_FORM_ALT_SOUNDCLOUD
in htdocs/class/textsanitizer/soundcloud/soundcloud.php lines 18-22,
_XOOPS_FORM_ALTWIKI in htdocs/class/textsanitizer/wiki/wiki.php lines 38-42,
_XOOPS_FORM_ALTWMP in htdocs/class/textsanitizer/wmp/wmp.php lines 39-45, and
_XOOPS_FORM_ALTYOUTUBE in htdocs/class/textsanitizer/youtube/youtube.php lines
31-37.
In `@htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php`:
- Around line 226-239: Update the toolbar rendering around the $sizes
initialization and the size dropdown call in the toolbar method so the Size
dropdown is appended only when $sizes contains valid options. Preserve rendering
of the Font and Color dropdowns regardless of whether formtextdhtml_sizes is
absent or empty.
- Around line 150-169: The compatibility path around renderCodeButtons() and
renderTypography() must ensure toolbar.css and toolbar.js are loaded even when
render() is bypassed. Update the retained protected helpers or add an explicit
shared asset-rendering method, and invoke it from those helper paths without
duplicating asset registration when render() is used.
In `@htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap4.php`:
- Around line 46-48: Escape the button text using escapeElementValue() instead
of appending getValue() directly in the Bootstrap 4 renderer’s button output.
Apply the same change in
htdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.php lines 47-49,
ensuring both renderers safely encode button values while preserving their
existing title and value attributes.
In `@tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php`:
- Around line 121-124: Escape the button caption derived from element values in
Bootstrap 4 and Bootstrap 5 renderFormButton and renderFormButtonTray
implementations. Then remove those four method entries from
KNOWN_TEXT_CONTEXT_GAPS so the containment test covers them.
- Around line 392-399: Update the parse method to capture the previous libxml
internal-error setting before calling libxml_use_internal_errors(true), then
restore that saved setting after libxml_clear_errors() and before returning the
DOMDocument.
In `@tests/unit/htdocs/class/xoopsform/XoopsFormRendererUniquenessTest.php`:
- Around line 97-102: The phpFiles scan in XoopsFormRendererUniquenessTest must
inspect every PHP file for renderer declarations; remove the filename-based
str_starts_with filter in the foreach loop, or replace it with a content-based
prefilter that still detects declarations such as xoopsformrenderer.php.
---
Outside diff comments:
In `@htdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.php`:
- Around line 407-415: Update jsCall’s argument-encoding branch to catch
\Throwable from json_encode, emit an E_USER_WARNING, and append a safe
empty-string argument when encoding fails. Preserve numeric handling and
successful JSON encoding so malformed toolbar labels do not abort form
rendering.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e9fdba3c-ab72-46a4-803f-2d3e06487815
📒 Files selected for processing (28)
htdocs/class/textsanitizer/mms/mms.phphtdocs/class/textsanitizer/mp3/mp3.phphtdocs/class/textsanitizer/readme.txthtdocs/class/textsanitizer/rtsp/rtsp.phphtdocs/class/textsanitizer/soundcloud/soundcloud.phphtdocs/class/textsanitizer/wiki/wiki.phphtdocs/class/textsanitizer/wmp/wmp.phphtdocs/class/textsanitizer/youtube/youtube.phphtdocs/class/xoopseditor/dhtmltextarea/XoopsDhtmlToolbar.phphtdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.csshtdocs/class/xoopseditor/dhtmltextarea/assets/toolbar.jshtdocs/class/xoopseditor/tinymce5/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap4.phphtdocs/class/xoopseditor/tinymce5/js/tinymce/plugins/xoopsimagemanager/xoopsimagemanager.phphtdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap4.phphtdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap5.phphtdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/xoopsimagemanager.phphtdocs/class/xoopsform/formdhtmltextarea.phphtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap3.phphtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap4.phphtdocs/class/xoopsform/renderer/XoopsFormRendererBootstrap5.phphtdocs/class/xoopsform/renderer/XoopsFormRendererLegacy.phphtdocs/class/xoopsform/renderer/XoopsFormRendererTailwind.phphtdocs/class/xoopsform/renderer/XoopsFormRendererValueEscapeTrait.phptests/bootstrap.phptests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarExtensionEscapingTest.phptests/unit/htdocs/class/xoopsform/XoopsDhtmlToolbarTest.phptests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.phptests/unit/htdocs/class/xoopsform/XoopsFormRendererUniquenessTest.php
💤 Files with no reviewable changes (3)
- htdocs/class/xoopseditor/tinymce5/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap4.php
- htdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap5.php
- htdocs/class/xoopseditor/tinymce7/js/tinymce/plugins/xoopsimagemanager/XoopsFormRendererBootstrap4.php
- Escape the localized title attribute in the seven TextSanitizer extension buttons (mms, mp3, rtsp, soundcloud, wiki, wmp, youtube); the constants were concatenated raw into a single-quoted attribute. - Omit the Size dropdown when no size options are configured instead of rendering a toggle that opens an empty menu; Font and Color still render, and the group aria-label drops the size segment accordingly. - Inject toolbar.css/toolbar.js from renderCodeButtons() and renderTypography() as well as render(), so a renderer subclass that composes the rows directly still gets working assets; the existing static guard keeps the injection single-shot per request. - jsCall(): degrade a json_encode() failure (malformed UTF-8 in a translated label) to a warning plus an empty-string argument instead of letting the exception abort the whole form render. - Tests: restore the previous libxml internal-errors setting in the escaping test's parse() helper; switch the renderer-uniqueness scan to a content prefilter so declarations in differently-named files are still found; pin asset state in the byte-identity toolbar tests; cover the direct-helper asset injection and the omitted Size dropdown.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (8)
tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php:196
- The containment assertions are inverted: this currently asserts the hostile render has at least as many elements/scripts/images as the benign baseline, which will not fail when injection adds nodes. To enforce containment, the hostile shape must be <= the baseline shape for counts (elements/scripts/images).
self::assertLessThanOrEqual($baseline['elements'], $shape['elements'], "An element was injected by $where");
self::assertLessThanOrEqual($baseline['scripts'], $shape['scripts'], "A <script> was injected by $where");
self::assertLessThanOrEqual($baseline['images'], $shape['images'], "An <img> was injected by $where");
htdocs/class/textsanitizer/youtube/youtube.php:36
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeYoutube("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTERYOUTUBEURL, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERHEIGHT, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERWIDTH, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALTYOUTUBE, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
htdocs/class/textsanitizer/mp3/mp3.php:36
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$buttonHtml = "<button type='button' class='btn btn-default' onclick='xoopsCodeMp3("
. json_encode((string) $textarea_id, $jsonFlags) . ");' title='"
. htmlspecialchars(_XOOPS_FORM_ALTMP3, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8') . "'>"
htdocs/class/textsanitizer/wmp/wmp.php:44
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeWmp("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTERWMPURL, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERHEIGHT, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERWIDTH, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALTWMP, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
htdocs/class/textsanitizer/mms/mms.php:44
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeMms("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTERMMSURL, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERHEIGHT, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERWIDTH, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALTMMS, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
htdocs/class/textsanitizer/rtsp/rtsp.php:44
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeRtsp("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTERRTSPURL, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERHEIGHT, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ALT_ENTERWIDTH, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALTRTSP, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
htdocs/class/textsanitizer/soundcloud/soundcloud.php:21
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeSoundCloud("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTER_SOUNDCLOUD_URL, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALT_SOUNDCLOUD, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
htdocs/class/textsanitizer/wiki/wiki.php:41
- Using JSON_THROW_ON_ERROR here can make toolbar rendering fatal if any argument contains invalid UTF-8 bytes (e.g., a mis-encoded translation constant). Since this value is used only to safely embed strings into an onclick handler, prefer JSON_INVALID_UTF8_SUBSTITUTE (non-throwing) so the extension degrades safely instead of crashing the request.
$jsonFlags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR;
$code = "<button type='button' class='btn btn-default btn-sm' onclick='xoopsCodeWiki("
. json_encode((string) $textarea_id, $jsonFlags) . ","
. json_encode(_XOOPS_FORM_ENTERWIKITERM, $jsonFlags)
. ");' onmouseover='style.cursor=\"hand\"' title='" . htmlspecialchars(_XOOPS_FORM_ALTWIKI, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
…owing Swap JSON_THROW_ON_ERROR for JSON_INVALID_UTF8_SUBSTITUTE in the seven extension button builders (mms, mp3, rtsp, soundcloud, wiki, wmp, youtube) and the readme pattern they follow. The values only feed string arguments in an onclick handler, so a mis-encoded translation constant should produce a substituted character in one button, not an uncaught JsonException that aborts the whole form render. Same failure class as the jsCall() hardening in the shared toolbar.
Contributor
Author
|
f7d13a9 addresses Copilot's latest suppressed comments (no open threads, so noting here): JSON_THROW_ON_ERROR in the seven TextSanitizer extension buttons — fixed. All seven (mms, mp3, rtsp, soundcloud, wiki, wmp, youtube) plus the readme pattern now use JSON_INVALID_UTF8_SUBSTITUTE: the values only feed string arguments in an onclick handler, so a mis-encoded translation constant now degrades to a substituted character in one button instead of an uncaught JsonException aborting the whole form render. Same failure class as the jsCall() hardening in 2b4da55. "Containment assertions are inverted" in XoopsFormRendererEscapingTest — not changed; the claim reads PHPUnit's parameter order backwards. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tests/unit/htdocs/class/xoopsform/XoopsFormRendererEscapingTest.php:196
- The injection-containment assertions in this test are inverted:
assertLessThanOrEqual($baseline, $shape)will pass when the hostile render adds elements/scripts/images (i.e., exactly when injection happened). This makes the suite unable to detect regressions in escaping.
Swap the operands so the hostile shape is required to be <= the benign baseline (no increases).
self::assertLessThanOrEqual($baseline['elements'], $shape['elements'], "An element was injected by $where");
self::assertLessThanOrEqual($baseline['scripts'], $shape['scripts'], "A <script> was injected by $where");
self::assertLessThanOrEqual($baseline['images'], $shape['images'], "An <img> was injected by $where");
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
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.
Summary by Sourcery
Introduce a shared, framework-neutral DHTML editor toolbar for XoopsFormDhtmlTextArea and update all form renderers to use it, while hardening related JavaScript helpers and test infrastructure.
New Features:
Enhancements:
Tests:
Summary by Gitar
XoopsDhtmlToolbarclass to unify editor markup across all renderers<details>dropdownsformdhtmltextarea.jsXoopsDhtmlToolbarTest.phpto verify renderer output parity and event contractsThis will update automatically on new commits.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Tests