feat(editor): add SCEditor as an optional BBCode editor - #152
Merged
Conversation
Registers SCEditor alongside the existing editors. dhtmltextarea remains the default, so nothing changes for an existing site until an administrator selects the new editor in preferences. It runs permanently in BBCode source mode and never in WYSIWYG. That is a safety property rather than a preference: in visual mode any tag the editor does not recognise -- [siteurl], [img id=], [[WikiPage]] and the smilie text codes held in the smiles table -- would be silently dropped when an existing post is opened and saved. In source mode nothing re-serialises the document, so stored content is never rewritten. MyTextSanitizer remains the only thing that parses BBCode, and no read path changes. js/xoops-bbcode.js teaches SCEditor the XOOPS dialect. Two defaults had to be overridden because they differ from XOOPS and would rewrite existing posts: strikethrough is [d], not [s], and [size=] takes the named XOOPS sizes (xx-small .. xx-large) rather than the numeric 1-7 scale. Tags that are off by default in the sanitiser are registered so existing content round-trips, but are kept off the toolbar. The plugin self-gates. editor_registry.php reports an empty order, and isActive() returns false, unless every required file is readable, so a missing or partial library leaves the editor out of the preferences list rather than offering a control that cannot work. Note that XoopsEditorHandler::getList() builds the list from the registry alone and never consults isActive(), which is why the gate lives in the registry. The library is vendored under minified/ following the layout of its upstream release, matching how tinymce and easymde are already carried in this repo. It is MIT licensed, which is compatible with GPL-2.0-or-later; the copyright notice is preserved inside the minified bundle.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @mambax7, your pull request is larger than the review limit of 150000 diff characters
|
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:
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 GuideAdds SCEditor as an optional BBCode editor, wiring it into the XOOPS editor registry, providing a PHP integration class and a JS plugin that encodes the XOOPS-specific BBCode dialect, while vendoring the SCEditor minified library and related assets with safety gating to ensure source-mode-only editing and correct behavior when the library is missing or partially installed. Sequence diagram for SCEditor initialization in BBCode source modesequenceDiagram
participant User
participant XOOPS_Form as XoopsForm
participant FormSCEditor
participant Browser
participant SCEditor as SCEditor_JS
User->>XOOPS_Form: submit form config using FormSCEditor
XOOPS_Form->>FormSCEditor: render()
FormSCEditor->>FormSCEditor: isActive()
FormSCEditor->>Browser: HTML textarea + script tags
Browser->>SCEditor: load sceditor.min.js
Browser->>SCEditor: load minified/formats/bbcode.js
Browser->>SCEditor: load js/xoops-bbcode.js
Browser->>SCEditor: sceditor.create(textarea, format="bbcode", toolbar=xoopsBBCodeToolbar, emoticonsEnabled=false)
Browser->>SCEditor: instance.sourceMode(true)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
============================================
+ 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:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds SCEditor as an optional XOOPS editor implementation, aiming to provide a BBCode source-mode editing experience while preserving XOOPS-specific BBCode semantics via a custom SCEditor dialect plugin and a registry-based availability gate.
Changes:
- Introduces a new
FormSCEditoreditor implementation and registers it witheditor_registry.php. - Vendors SCEditor minified assets under
minified/and adds XOOPS BBCode integration glue (js/xoops-bbcode.js). - Adds install/documentation and standard directory
index.phpguards.
Reviewed changes
Copilot reviewed 12 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| htdocs/class/xoopseditor/sceditor/sceditor.php | New SCEditor-backed editor class that renders a textarea and initializes SCEditor. |
| htdocs/class/xoopseditor/sceditor/editor_registry.php | Registers the editor and gates visibility in the preference list based on required files. |
| htdocs/class/xoopseditor/sceditor/INSTALL.md | Installation/documentation for SCEditor availability and file layout. |
| htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js | XOOPS-authored SCEditor BBCode dialect + toolbar/command configuration. |
| htdocs/class/xoopseditor/sceditor/language/english.php | Adds the editor title language constant. |
| htdocs/class/xoopseditor/sceditor/language/index.php | Directory guard returning 404. |
| htdocs/class/xoopseditor/sceditor/index.php | Directory guard returning 404. |
| htdocs/class/xoopseditor/sceditor/js/index.php | Directory guard returning 404. |
| htdocs/class/xoopseditor/sceditor/css/index.php | Directory guard returning 404. |
| htdocs/class/xoopseditor/sceditor/minified/sceditor.min.js | Vendored SCEditor core JS bundle. |
| htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js | Vendored SCEditor BBCode format implementation. |
| htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js | Vendored SCEditor XHTML format implementation. |
| htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css | Vendored SCEditor default theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css | Vendored SCEditor default dark theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/modern.min.css | Vendored SCEditor modern theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/office.min.css | Vendored SCEditor office theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/office-toolbar.min.css | Vendored SCEditor office-toolbar theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/square.min.css | Vendored SCEditor square theme CSS. |
| htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css | Vendored SCEditor content CSS for the editing area. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/format.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js | Vendored SCEditor plugin. |
| htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js | Vendored SCEditor compatibility plugin. |
| htdocs/class/xoopseditor/sceditor/minified/icons/material.js | Vendored SCEditor icon pack. |
| htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js | Vendored SCEditor icon pack. |
Files not reviewed (17)
- htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/material.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/format.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/modern.min.css: Generated file
Comment on lines
+399
to
+412
| sceditor.command.set('siteurl', { | ||
| txtExec: function (caller) { | ||
| var path = window.prompt('Site-relative path:', ''); | ||
| if (path) { | ||
| this.insertText('[siteurl=' + path + ']', '[/siteurl]'); | ||
| } | ||
| }, | ||
| tooltip: 'Site URL' | ||
| }); | ||
|
|
||
| sceditor.command.set('quote', { | ||
| txtExec: ['[quote]', '[/quote]'], | ||
| tooltip: 'Quote' | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
Fixed in eaef083: a source-mode email command override now inserts the bare [email]address[/email] form that module.textsanitizer.php decodes, replacing the stock [email=address]label[/email] behavior.
Greptile SummaryThe PR adds SCEditor as an optional BBCode source editor while preserving the existing default editor.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the prior BBCode rewrite issue is fixed because SCEditor starts in source mode, initializes the source textarea without conversion, and blocks subsequent transitions to WYSIWYG.
|
| Filename | Overview |
|---|---|
| htdocs/class/xoopseditor/sceditor/sceditor.php | Implements the editor lifecycle, asset loading, source-only initialization, dimension normalization, and validation; the previous initialization-rewrite issue is fixed. |
| htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js | Defines XOOPS-specific BBCode formats and source-mode toolbar commands without introducing an eligible follow-up defect. |
| htdocs/class/xoopseditor/sceditor/editor_registry.php | Registers SCEditor only when all required bundled assets are readable. |
Reviews (6): Last reviewed commit: "fix(sceditor): reject non-finite and neg..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Address review findings on the SCEditor integration: - Create the instance with startInSourceMode instead of forcing source mode after creation. The default WYSIWYG init parsed existing BBCode into HTML and re-serialised it on the switch back - exactly the round-trip this plugin promises never happens - which could rewrite or drop XOOPS-specific tags such as [siteurl] before the user typed anything. - Register dialect overrides under SCEditor's actual BBCode registry keys, which are the literal tag names: b, i, u, url, and d for the XOOPS strikethrough tag. The previous bold/italic/underline/link/ strike keys created new bogus tags instead of overriding the stock definitions, so existing [d] content was not recognised on any conversion path. - Drop the [wikipage] format registration: the [[...]] wiki syntax has no tag name SCEditor's grammar can express, and the registration only invented a fake [wikipage] tag. The toolbar command that inserts [[...]] directly remains. - Override the stock email command, which inserts the [email=address]label[/email] attribute form. XOOPS only decodes the bare [email]address[/email] form, so the button would have produced BBCode that publishes as literal text. - Rewrite INSTALL.md to match the repository layout: the SCEditor distribution ships bundled under minified/ (the paths isActive() and render() actually load), replacing instructions that told admins to download the library into js/ and css/ paths XOOPS never reads.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 34 changed files in this pull request and generated no new comments.
Files not reviewed (17)
- htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/material.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/format.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/modern.min.css: Generated file
Suppressed comments (4)
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:142
- The overridden
[url]tag renders an<a href="...">using the raw attribute value. If SCEditor is ever switched into WYSIWYG/preview mode, this can reintroduce unsafe schemes (e.g.javascript:) and diverges from SCEditor’s own bbcode format implementation, which runs links throughescapeUriScheme()/ entity escaping.
html: function (token, attrs, content) {
var href = (attrs && attrs.defaultattr) || '';
return '<a href="' + href + '">' + content + '</a>';
}
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:155
- The
[siteurl]BBCode handler’sformat()readsdata-siteurl, buthtml()never sets that attribute (it only setshref). If the BBCode↔HTML conversion path ever runs, existing[siteurl=...]content will serialize back to[siteurl=]...[/siteurl](empty path) and lose the link target.
format: function (element, content) {
return '[siteurl=' + (element.getAttribute('data-siteurl') || '') + ']' + content + '[/siteurl]';
},
html: function (token, attrs, content) {
var path = (attrs && attrs.defaultattr) || '';
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:204
- The
[font]override is internally inconsistent:format()expects a<font face="...">element, buthtml()generates a<span style="font-family: ...">. If the BBCode↔HTML conversion path ever runs,[font=...]will not round-trip and can collapse to[font=]...[/font](empty face).
format: function (element, content) {
var face = element.getAttribute ? element.getAttribute('face') : '';
return '[font=' + face + ']' + content + '[/font]';
},
html: function (token, attrs, content) {
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:219
- The
[color]override is internally inconsistent:format()expects a<font color="...">element, buthtml()generates a<span style="color: ...">. If the BBCode↔HTML conversion path ever runs,[color=...]will not round-trip and can collapse to[color=]...[/color](empty color).
format: function (element, content) {
var color = element.getAttribute ? element.getAttribute('color') : '';
return '[color=' + color + ']' + content + '[/color]';
},
html: function (token, attrs, content) {
Follow-up to the review pass. The format/html definitions never run while the editor stays in source mode, but the file's stated contract is that a future WYSIWYG/preview toggle must not inherit a wrong conversion table, so make them actually correct: - url: entity-escape and scheme-check the href in html(), matching the stock format's handling, so an unsafe scheme cannot become a live link if the conversion path ever runs. - siteurl: html() now stores the path on the data-siteurl attribute that format() reads back, so the tag no longer loses its target on a round-trip; the path is escaped for the attribute context. - font/color: claim the span style shapes (styles:) that their own html() emits, and let format() read the attribute or the style, so both tags round-trip instead of collapsing to an empty value. - size: add the font-size styles claim - without it no HTML element ever mapped back to [size=] at all. - Escape the URL-bearing content interpolated into src/href attributes across the media tags (img, youtube, iframe, mp3, soundcloud, mms, rtsp, wmp) - the same attribute-context class as the url finding. - Drop the dead escaped $height assignment in render(); the create options use the raw config value via json_encode(). Escaping uses the sceditor.escapeEntities / escapeUriScheme helpers the bundled core exports, with local fallbacks so the file stays loadable against a stripped build.
Contributor
Author
|
11200ce addresses the remaining reviewer output that had no open threads: Copilot's four suppressed comments on
These definitions stay inert while the editor is in source mode (which Scrutinizer — the dead escaped |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 34 changed files in this pull request and generated no new comments.
Files not reviewed (17)
- htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/material.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/format.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/modern.min.css: Generated file
Suppressed comments (2)
htdocs/class/xoopseditor/sceditor/sceditor.php:144
- The PR description says SCEditor must never run in WYSIWYG mode (to avoid BBCode↔HTML round-trips), but the toolbar is taken from the global
xoopsBBCodeToolbar. If that global is overridden to include thesourcecommand, users can toggle into WYSIWYG and trigger the unsafe conversion path. To enforce the safety property, explicitly exclude thesourcecommand via SCEditor options (and keep startInSourceMode).
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:174 siteurl'shtml()uses the raw default attribute directly ashref(after entity escaping), without SCEditor’s scheme check. If any BBCode↔HTML conversion path runs (e.g. future preview/WYSIWYG toggle),[siteurl=javascript:...]could become a livejavascript:link. UseescapeUriScheme()for thehref, while keepingdata-siteurlas the original value for round-tripping.
html: function (token, attrs, content) {
var path = escapeEntities((attrs && attrs.defaultattr) || '');
return '<a data-siteurl="' + path + '" href="' + path + '">' + content + '</a>';
}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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/sceditor/css/index.php`:
- Around line 1-3: Add the standard XOOPS copyright header block at the
beginning of this guard file, before the PHP response logic; apply the same
header consistently to the sibling index.php guard files introduced in this
change.
In `@htdocs/class/xoopseditor/sceditor/index.php`:
- Around line 1-3: Add the repository’s standard XOOPS copyright header before
the executable code in htdocs/class/xoopseditor/sceditor/index.php lines 1-3,
htdocs/class/xoopseditor/sceditor/js/index.php lines 1-3, and
htdocs/class/xoopseditor/sceditor/language/index.php lines 1-3; preserve the
existing http_response_code(404) and exit behavior in each entry point.
In `@htdocs/class/xoopseditor/sceditor/INSTALL.md`:
- Line 12: Update the directory-layout fenced code block in INSTALL.md to
specify the text language tag, changing the opening fence to ```text while
preserving the diagram content and rendered output.
In `@htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js`:
- Around line 78-81: Rename nearestXoopsSize to toXoopsSize and update every
call site to reflect that it performs exact XOOPS size conversion with a medium
fallback. Do not change the conversion behavior or introduce nearest-match
logic.
- Around line 428-508: Update the SCEditor command definitions in this diff to
read prompt labels and tooltip text from a PHP-provided global, following the
existing window.xoopsBBCodeToolbar publication pattern and the editor language
symbols in sceditor.php. Replace each hardcoded English string used by email,
size, siteurl, image, youtube, and wikipage with the corresponding translated
value; preserve command behavior, and use SCEditor's dropdown API for size
selection if the existing integration supports it instead of window.prompt.
- Around line 60-62: Update the escapeUriScheme fallback in xoops-bbcode.js to
reject unsafe URI values rather than returning input unchanged: allow only
approved schemes and relative references, and return a safe non-link value for
disallowed schemes such as javascript:. Keep the existing
sceditor.escapeUriScheme implementation preferred when available and preserve
all html() definitions’ scheme validation.
- Around line 199-209: Update the `code` definition’s `html` function to
preserve the parsed language by emitting it as a `data-lang` attribute on the
generated `<code>` element, following the existing `siteurl`/`data-siteurl`
pattern. Keep bare `<code>` output when no language attribute is present.
In `@htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js`:
- Line 2: In the command templates object, update the rtl and ltr definitions to
emit the direction declaration using the correctly spelled style attribute
instead of stlye. Preserve their existing direction values and wrapper
structure.
In `@htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js`:
- Around line 1-2: Replace the header comment in the autosave.js vendored asset
with the complete MIT license text for SCEditor 3.2.1, preserving the minified
implementation unchanged and ensuring the license is included directly with the
asset rather than only referencing sceditor.com/license.
In `@htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js`:
- Around line 1-2: Confirm whether the SCEditor plugin assets, including
autoyoutube, are intentionally retained despite not being loaded by
sceditor.php; remove the unused minified plugin files if they are not planned
for integration, or document their intended purpose. Do not modify the inherited
innerHTML usage in the minified vendored sources.
In `@htdocs/class/xoopseditor/sceditor/sceditor.php`:
- Around line 92-93: Remove the dead $configs['width'] fallback and use
$this->width directly when computing $width. Apply the same simplification to
the height expressions at the locations corresponding to the width and height
attributes, using $this->height directly while preserving the existing escaping
behavior.
- Around line 43-44: In htdocs/class/xoopseditor/sceditor/sceditor.php:43-44,
add setWidth() and setHeight() methods that normalize incoming values before
assigning the typed properties, allowing XoopsEditor::__construct() to route
configuration through the setters. In
htdocs/class/xoopseditor/sceditor/sceditor.php:92-93, remove the $configs
width/height fallback usages at lines 93, 143, and 144 and read the normalized
properties directly.
- Around line 38-40: Complete the FormSCEditor class docblock by adding the
required `@category`, `@package`, `@author`, `@copyright`, `@license`, and `@link` tags,
using the project’s established metadata values and formatting conventions.
- Around line 165-168: Update the generated validation JavaScript in the method
containing $eltmsg and $eltname: encode both the element identifier and alert
message with json_encode using JSON_THROW_ON_ERROR, rather than manual quote
replacement or raw interpolation. Store the result of document.getElementById in
a local element variable and guard it before reading .value, preserving
validation when the element is present and avoiding an exception when absent.
- Around line 135-145: Update the SCEditor initialization around sceditor.create
to remove the source control from the toolbar before passing it to the editor,
then retrieve the created instance with sceditor.instance(el) and override its
sourceMode method so false cannot enable WYSIWYG mode. Preserve the configured
toolbar’s other controls and keep the editor permanently in BBCode source mode.
🪄 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: a9786478-3f13-46c1-a51b-b72e8d8fbf4d
⛔ Files ignored due to path filters (12)
htdocs/class/xoopseditor/sceditor/minified/jquery.sceditor.bbcode.min.jsis excluded by!**/*.min.js,!**/*.min.jshtdocs/class/xoopseditor/sceditor/minified/jquery.sceditor.min.jsis excluded by!**/*.min.js,!**/*.min.jshtdocs/class/xoopseditor/sceditor/minified/jquery.sceditor.xhtml.min.jsis excluded by!**/*.min.js,!**/*.min.jshtdocs/class/xoopseditor/sceditor/minified/sceditor.min.jsis excluded by!**/*.min.js,!**/*.min.jshtdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/default.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/famfamfam.pngis excluded by!**/*.pnghtdocs/class/xoopseditor/sceditor/minified/themes/modern.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/office-toolbar.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/office.min.cssis excluded by!**/*.min.css,!**/*.min.csshtdocs/class/xoopseditor/sceditor/minified/themes/square.min.cssis excluded by!**/*.min.css,!**/*.min.css
📒 Files selected for processing (22)
htdocs/class/xoopseditor/sceditor/INSTALL.mdhtdocs/class/xoopseditor/sceditor/css/index.phphtdocs/class/xoopseditor/sceditor/editor_registry.phphtdocs/class/xoopseditor/sceditor/index.phphtdocs/class/xoopseditor/sceditor/js/index.phphtdocs/class/xoopseditor/sceditor/js/xoops-bbcode.jshtdocs/class/xoopseditor/sceditor/language/english.phphtdocs/class/xoopseditor/sceditor/language/index.phphtdocs/class/xoopseditor/sceditor/minified/formats/bbcode.jshtdocs/class/xoopseditor/sceditor/minified/formats/xhtml.jshtdocs/class/xoopseditor/sceditor/minified/icons/material.jshtdocs/class/xoopseditor/sceditor/minified/icons/monocons.jshtdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.jshtdocs/class/xoopseditor/sceditor/minified/plugins/autosave.jshtdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.jshtdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.jshtdocs/class/xoopseditor/sceditor/minified/plugins/emojis.jshtdocs/class/xoopseditor/sceditor/minified/plugins/format.jshtdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.jshtdocs/class/xoopseditor/sceditor/minified/plugins/undo.jshtdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.jshtdocs/class/xoopseditor/sceditor/sceditor.php
Comment on lines
+1
to
+3
| <?php | ||
| http_response_code(404); | ||
| exit; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add the XOOPS copyright header block to this guard file.
The coding guidelines require every PHP source file to begin with the XOOPS copyright header block. This directory guard omits it. The same applies to the sibling index.php guards added in this PR.
♻️ Proposed header
<?php
+/**
+ * SCEditor (BBCode) Editor for XOOPS
+ *
+ * `@copyright` (c) 2000-2026 XOOPS Project (https://xoops.org)
+ * `@license` GNU GPL 2 (https://www.gnu.org/licenses/gpl-2.0.html)
+ * `@package` class
+ * `@subpackage` editor
+ * `@since` 2.8.0
+ * `@author` XOOPS Development Team
+ */
http_response_code(404);
exit;As per coding guidelines: "Every source file begins with the XOOPS copyright header block".
📝 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.
Suggested change
| <?php | |
| http_response_code(404); | |
| exit; | |
| <?php | |
| /** | |
| * SCEditor (BBCode) Editor for XOOPS | |
| * | |
| * `@copyright` (c) 2000-2026 XOOPS Project (https://xoops.org) | |
| * `@license` GNU GPL 2 (https://www.gnu.org/licenses/gpl-2.0.html) | |
| * `@package` class | |
| * `@subpackage` editor | |
| * `@since` 2.8.0 | |
| * `@author` XOOPS Development Team | |
| */ | |
| http_response_code(404); | |
| exit; |
🤖 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/sceditor/css/index.php` around lines 1 - 3, Add the
standard XOOPS copyright header block at the beginning of this guard file,
before the PHP response logic; apply the same header consistently to the sibling
index.php guard files introduced in this change.
Source: Coding guidelines
Contributor
Author
There was a problem hiding this comment.
Not changed: every sibling editor's guard stub (dhtmltextarea/index.php, textarea/index.php, ...) is the bare '<?php http_response_code(404); exit;' with no header — these three follow that established core convention. Adding headers only here would make the sceditor directory the odd one out; a header sweep across all guard stubs would be its own change.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- SCEditor guard files ---'
fd -t f -p 'htdocs/class/xoopseditor/sceditor' 'index.php' -x sh -c '
printf "\n%s\n" "--- $1"
sed -n "1,15p" "$1"
' sh {}
printf '%s\n' '--- Selected sibling editor guard files ---'
for file in \
htdocs/class/xoopseditor/dhtmltextarea/index.php \
htdocs/class/xoopseditor/textarea/index.php
do
if [ -f "$file" ]; then
printf "\n%s\n" "--- $file"
sed -n '1,15p' "$file"
fi
done
printf '%s\n' '--- All direct editor guard files: first non-empty lines ---'
fd -t f -d 2 '^index\.php$' htdocs/class/xoopseditor -x sh -c '
printf "\n%s\n" "--- $1"
sed -n "1,8p" "$1"
' sh {}Length of output: 1106
The first command used the fd arguments in the wrong order. I need one corrected read-only inspection before I can evaluate the convention.
You are interacting with an AI system.
Third review pass on the SCEditor integration: - Localize every toolbar command label and prompt: the editor language file gains _XOOPS_EDITOR_SCEDITOR_* constants, sceditor.php publishes them as window.xoopsSCEditorLang before js/xoops-bbcode.js loads, and the commands read them with English fallbacks for missing keys. - Add setWidth()/setHeight() normalizing setters. XoopsEditor's constructor routes config keys through set*() methods, so an int width previously hit a TypeError on the typed property; bare numbers now become pixel lengths. render() reads the normalized properties directly, dropping the dead $configs fallbacks (width/height never land in $this->configs precisely because the constructor routes them to properties). - Harden the escapeUriScheme fallback: instead of passing values through unchanged when a stripped build omits the core helper, it now allows relative references and approved schemes only, and returns a dead fragment for anything else. - [code=lang]: html() stores the parsed language on data-lang (the attribute format() reads back), so the language survives a round-trip. - Rename nearestXoopsSize to toXoopsSize - it does an exact-match conversion with a medium fallback, not a nearest match. - renderValidationJS(): encode the element id and message with json_encode instead of manual quote escaping, and guard the getElementById result before reading .value. - Guard instance.sourceMode against a stray sourceMode(false) call so outside script cannot flip the editor into the WYSIWYG conversion path; the getter and sourceMode(true) behave as before. - Ship the upstream MIT license text verbatim as minified/LICENSE.md; document that the unloaded minified extras (plugins, icons, xhtml format, extra themes) are retained deliberately so upgrades stay a wholesale minified/ replacement; tag the INSTALL.md layout fence as text; complete the FormSCEditor docblock.
All _XOOPS_EDITOR_SCEDITOR_* constants introduced by the editor's language file, including the registry title from the initial commit, listed under a new 2.7.3 section.
Independent adversarial review findings, each reproduced against the vendored SCEditor 3.2.1 before fixing: - Also guard toggleSourceMode(), the method SCEditor's own source command calls; wrapping only sourceMode() left an unlocked door into the WYSIWYG conversion path. The override re-enters source mode when anything tries to leave it. - Drop the RegExp attribute constraint from the email registry entry. SCEditor accepts only null or an array there and calls .includes() on the value, so the RegExp threw a TypeError for EVERY anchor during HTML-to-BBCode conversion. A mailto anchor now serialises via the url handler as [url=mailto:...], which MyTextSanitizer decodes. - Make the claimed round-trips actually round-trip: siteurl claims its anchors via a data-siteurl attribute match (instead of losing them to url), img re-emits width/float so format() can rebuild them, and youtube carries tag identity and dimensions in data attributes. - Validate width/height as a single plain CSS length in the setters. HTML escaping alone let a configured value smuggle extra style declarations (display:none, a background-image beacon) into the textarea style attribute. - ENT_SUBSTITUTE on the rendered name/value/width and JSON_INVALID_UTF8_SUBSTITUTE for the JS id: one invalid UTF-8 byte previously rendered an EMPTY textarea (saving would erase the original content) or threw a JsonException mid-render. - Gate activation on the two theme stylesheets as well as the scripts, in both isActive() and editor_registry.php, so a half-removed library cannot leave a selectable but unstyled editor; INSTALL.md updated to match.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/sceditor/editor_registry.php`:
- Around line 27-31: Update the $sceditorInstalled asset gate so every required
asset path in the existing checks must satisfy both is_file() and is_readable(),
preventing directories from being accepted as installed JavaScript or CSS
assets.
In `@htdocs/class/xoopseditor/sceditor/INSTALL.md`:
- Around line 36-39: Update the SCEditor installation/removal instructions near
the FormSCEditor::isActive() and editor_registry.php asset-gate documentation to
include the supported step for clearing the editorlist cache after adding or
removing any required asset, ensuring the editor selection list is re-evaluated
immediately.
In `@htdocs/class/xoopseditor/sceditor/minified/LICENSE.md`:
- Line 1: Configure markdownlint to exempt the vendored
htdocs/class/xoopseditor/sceditor/minified/LICENSE.md file from MD041, using the
repository’s existing per-file suppression or ignore configuration. Leave the
MIT License content unchanged and avoid modifying the license text itself.
In `@htdocs/class/xoopseditor/sceditor/sceditor.php`:
- Around line 53-76: Update the PHPDoc blocks for setWidth() and setHeight() to
include `@throws` entries documenting the actual exception type raised by
normalizeCssLength(). Keep the existing `@param` and `@return` documentation
unchanged.
🪄 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: 0d54d1fc-d02e-48cd-8e9f-37bff70196c0
📒 Files selected for processing (7)
docs/lang_diff.txthtdocs/class/xoopseditor/sceditor/INSTALL.mdhtdocs/class/xoopseditor/sceditor/editor_registry.phphtdocs/class/xoopseditor/sceditor/js/xoops-bbcode.jshtdocs/class/xoopseditor/sceditor/language/english.phphtdocs/class/xoopseditor/sceditor/minified/LICENSE.mdhtdocs/class/xoopseditor/sceditor/sceditor.php
| @@ -0,0 +1,22 @@ | |||
| MIT License | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve MD041 without changing the vendored license text.
markdownlint-cli2 reports MD041 because MIT License is not a Markdown heading. Do not prepend # if this file must remain verbatim. Exempt this vendored license file from MD041 or configure a per-file suppression outside the license content.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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/sceditor/minified/LICENSE.md` at line 1, Configure
markdownlint to exempt the vendored
htdocs/class/xoopseditor/sceditor/minified/LICENSE.md file from MD041, using the
repository’s existing per-file suppression or ignore configuration. Leave the
MIT License content unchanged and avoid modifying the license text itself.
Source: Linters/SAST tools
Contributor
Author
There was a problem hiding this comment.
Not changed: this repository has no markdownlint configuration and no workflow that runs markdownlint, so MD041 is not enforced anywhere — adding suppression config for a linter that does not exist here would be dead weight. The file stays verbatim upstream (samclarke/SCEditor LICENSE.md), which also keeps future re-vendoring a plain copy.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate Markdown lint configuration files ---'
fd -HI -t f '(^|/)(\.markdownlint(\.json|\.yaml|\.yml|\.jsonc)?|markdownlint\.(json|yaml|yml|jsonc)|\.markdownlint-cli2\.(jsonc|yaml|yml))$' . || true
printf '%s\n' '--- Markdown lint references in tracked configuration and automation files ---'
rg -n -i -C 2 \
-g '!.git/**' \
-g '*.yml' -g '*.yaml' -g '*.json' -g '*.jsonc' \
-g 'Makefile' -g 'package.json' -g 'composer.json' \
'markdownlint|markdownlint-cli2|MD041' . || true
printf '%s\n' '--- GitHub workflow files ---'
fd -HI -t f . .github 2>/dev/null || trueLength of output: 1457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '45,70p' .coderabbit.yamlLength of output: 891
@mambax7, the repository has no standalone Markdownlint configuration or GitHub Actions workflow. However, .coderabbit.yaml explicitly enables CodeRabbit’s markdownlint tool at Lines 59-60.
Therefore, MD041 is enforced during CodeRabbit review. I cannot withdraw the finding on the stated basis.
The license must remain verbatim. A suppression or exclusion outside htdocs/class/xoopseditor/sceditor/minified/LICENSE.md remains the appropriate minimal fix.
You are interacting with an AI system.
Comment on lines
+53
to
+76
| /** | ||
| * Normalize a configured width before it reaches the typed property. | ||
| * XoopsEditor::__construct() routes config keys through set*() methods when they exist, | ||
| * so without this a caller passing an int (e.g. 400) would hit a TypeError. | ||
| * | ||
| * @param mixed $width CSS length or bare number (treated as pixels) | ||
| * | ||
| * @return void | ||
| */ | ||
| public function setWidth($width): void | ||
| { | ||
| $this->width = $this->normalizeCssLength($width, $this->width); | ||
| } | ||
|
|
||
| /** | ||
| * Normalize a configured height before it reaches the typed property. | ||
| * | ||
| * @param mixed $height CSS length or bare number (treated as pixels) | ||
| * | ||
| * @return void | ||
| */ | ||
| public function setHeight($height): void | ||
| { | ||
| $this->height = $this->normalizeCssLength($height, $this->height); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required @throws tags.
setWidth() and setHeight() are new public methods. Their PHPDoc blocks omit @throws.
Document the actual exception contract for both methods. As per coding guidelines: "New public methods have PHPDoc with @param, @return, and @throws tags".
🤖 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/sceditor/sceditor.php` around lines 53 - 76, Update
the PHPDoc blocks for setWidth() and setHeight() to include `@throws` entries
documenting the actual exception type raised by normalizeCssLength(). Keep the
existing `@param` and `@return` documentation unchanged.
Source: Coding guidelines
Contributor
Author
There was a problem hiding this comment.
Not changed: normalizeCssLength() has no throwing path — it is preg_match() on a literal pattern plus arithmetic coercion, and neither raises an exception. A @throws tag on setWidth()/setHeight() would document an exception the code cannot produce.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 36 changed files in this pull request and generated no new comments.
Files not reviewed (16)
- htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/material.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/format.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css: Generated file
Suppressed comments (7)
htdocs/class/xoopseditor/sceditor/sceditor.php:94
- normalizeCssLength() treats any numeric string as valid and appends "px" to the raw trimmed string. Values like "100." become "100.px" (invalid CSS) and scientific notation like "1e3" becomes "1e3px". Restrict numeric strings to plain decimal forms and normalize them before appending "px".
htdocs/class/xoopseditor/sceditor/sceditor.php:205 - SCEditor’s own defaults show
autoUpdateis disabled by default. Without enabling it (or otherwise syncing), the original<textarea>value can lag behind what the user typed in SCEditor, which can break required-field validation and/or submit stale content. EnableautoUpdate: truewhen creating the instance.
htdocs/class/xoopseditor/sceditor/sceditor.php:299 - renderValidationJS() checks
textarea.value, but when SCEditor is attached it may maintain its own source editor and only sync back to the original textarea whenautoUpdateis enabled. Use the SCEditor instance value when available, falling back to the textarea value when SCEditor isn’t running.
htdocs/class/xoopseditor/sceditor/sceditor.php:30 - The header says
@since 2.8.0, but docs/lang_diff.txt records this editor being added for version 2.7.3. Update the@sincetag so it matches the actual release this lands in.
This issue also appears in the following locations of the same file:
- line 92
- line 201
- line 297
htdocs/class/xoopseditor/sceditor/language/english.php:8
- This language file is listed in docs/lang_diff.txt under version 2.7.3, but its header says
@since 2.8.0. Align the@sinceversion with the documented release.
* @copyright (c) 2000-2026 XOOPS Project (https://xoops.org)
* @license GNU GPL 2 (https://www.gnu.org/licenses/gpl-2.0.html)
* @author XOOPS Development Team
* @since 2.8.0
* @package xoopseditor
*/
htdocs/class/xoopseditor/sceditor/editor_registry.php:16
- This registry header says
@since 2.8.0, but the change is documented for version 2.7.3 (docs/lang_diff.txt). Align the@sincetag with the actual release.
* @copyright (c) 2000-2026 XOOPS Project (https://xoops.org)
* @license GNU GPL 2 (https://www.gnu.org/licenses/gpl-2.0.html)
* @package class
* @subpackage editor
* @since 2.8.0
* @author XOOPS Development Team
* @see https://github.com/samclarke/SCEditor
htdocs/class/xoopseditor/sceditor/js/xoops-bbcode.js:493
- The size prompt only accepts an exact, case-sensitive match in
XOOPS_SIZESand doesn’t trim whitespace, so inputs like "Medium" or "medium " are silently ignored. Trimming + lowercasing keeps validation strict while reducing user error.
var choice = window.prompt(L('sizePrompt', 'Size (%s):').replace('%s', XOOPS_SIZES.join(', ')), 'medium');
if (choice && XOOPS_SIZES.indexOf(choice) !== -1) {
this.insertText('[size=' + choice + ']', '[/size]');
}
Fourth review pass:
- autoUpdate: true on the instance. SCEditor syncs the original
textarea on its own submit listener, but XOOPS validation runs from
the form's inline onsubmit attribute, which can fire first - required
-field validation could read a stale value without continuous sync.
- normalizeCssLength(): coerce numeric strings arithmetically before
appending px; is_numeric() also accepts '100.' and '1e3', which
previously concatenated into invalid CSS ('100.px', '1e3px').
- Activation gates check is_file() as well as is_readable() in both
isActive() and editor_registry.php - is_readable() alone accepts a
directory of the same name.
- Size prompt input is trimmed and lowercased before validation, so
'Medium' and 'medium ' insert instead of being silently ignored.
- INSTALL.md documents clearing the cached editor list (XoopsCache key
editorlist) after adding or removing required assets.
- @SInCE corrected to 2.7.3 in the three headers, matching the release
recorded in docs/lang_diff.txt.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 36 changed files in this pull request and generated no new comments.
Files not reviewed (16)
- htdocs/class/xoopseditor/sceditor/minified/formats/bbcode.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/formats/xhtml.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/material.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/icons/monocons.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/alternative-lists.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autosave.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/autoyoutube.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/dragdrop.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/emojis.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/format.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/plaintext.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/undo.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/plugins/v1compat.js: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/content/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/default.min.css: Generated file
- htdocs/class/xoopseditor/sceditor/minified/themes/defaultdark.min.css: Generated file
There was a problem hiding this comment.
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 (1)
htdocs/class/xoopseditor/sceditor/INSTALL.md (1)
69-76: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLink directly to the SCEditor release archive.
The plain URL contains a space before
releases, so CommonMark renders the link as the repository URL and leavesreleasesas plain text. Use a Markdown link tohttps://github.com/samclarke/SCEditor/releases.🤖 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/sceditor/INSTALL.md` around lines 69 - 76, Update the “Upgrading the bundled library” documentation to replace the malformed plain SCEditor URL with a Markdown link targeting https://github.com/samclarke/SCEditor/releases, while preserving the surrounding upgrade instructions and npm package reference.
🤖 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/sceditor/sceditor.php`:
- Around line 93-96: Update the dimension-normalization branch around the
numeric coercion to validate that the trimmed value is numeric, finite, and
non-negative before appending “px”. Reject INF, NAN, non-finite numeric strings,
and negative values by preserving the existing dimension fallback; only
normalize and return the pixel value for valid finite non-negative inputs.
---
Outside diff comments:
In `@htdocs/class/xoopseditor/sceditor/INSTALL.md`:
- Around line 69-76: Update the “Upgrading the bundled library” documentation to
replace the malformed plain SCEditor URL with a Markdown link targeting
https://github.com/samclarke/SCEditor/releases, while preserving the surrounding
upgrade instructions and npm package reference.
🪄 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: 7292b4e6-1988-4ec6-8909-b31dcf3fc981
📒 Files selected for processing (5)
htdocs/class/xoopseditor/sceditor/INSTALL.mdhtdocs/class/xoopseditor/sceditor/editor_registry.phphtdocs/class/xoopseditor/sceditor/js/xoops-bbcode.jshtdocs/class/xoopseditor/sceditor/language/english.phphtdocs/class/xoopseditor/sceditor/sceditor.php
- normalizeCssLength(): guard the coerced number before appending px.
is_numeric() accepts '1e400', which overflows to INF ('INFpx'), an
INF/NAN float can arrive directly, and a negative length is not a
usable dimension - all of those now keep the fallback value. The
guard reads the original value rather than the string round-trip:
(string) INF is 'INF', which casts back to 0.0 and would have slipped
through only to fatal on the later 0 + 'INF' coercion.
- INSTALL.md: make the upgrade pointer a proper Markdown link to the
SCEditor releases page instead of a bare URL running into the word
'releases'.
mambax7
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
August 7, 2026 13:14
All findings addressed or answered in threads; CI green
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.
Registers SCEditor alongside the existing editors. dhtmltextarea remains the default, so nothing changes for an existing site until an administrator selects the new editor in preferences.
It runs permanently in BBCode source mode and never in WYSIWYG. That is a safety property rather than a preference: in visual mode any tag the editor does not recognise -- [siteurl], [img id=], [[WikiPage]] and the smilie text codes held in the smiles table -- would be silently dropped when an existing post is opened and saved. In source mode nothing re-serialises the document, so stored content is never rewritten. MyTextSanitizer remains the only thing that parses BBCode, and no read path changes.
js/xoops-bbcode.js teaches SCEditor the XOOPS dialect. Two defaults had to be overridden because they differ from XOOPS and would rewrite existing posts: strikethrough is [d], not [s], and [size=] takes the named XOOPS sizes (xx-small .. xx-large) rather than the numeric 1-7 scale. Tags that are off by default in the sanitiser are registered so existing content round-trips, but are kept off the toolbar.
The plugin self-gates. editor_registry.php reports an empty order, and isActive() returns false, unless every required file is readable, so a missing or partial library leaves the editor out of the preferences list rather than offering a control that cannot work. Note that XoopsEditorHandler::getList() builds the list from the registry alone and never consults isActive(), which is why the gate lives in the registry.
The library is vendored under minified/ following the layout of its upstream release, matching how tinymce and easymde are already carried in this repo. It is MIT licensed, which is compatible with GPL-2.0-or-later; the copyright notice is preserved inside the minified bundle.
Summary by Sourcery
Integrate SCEditor as an optional BBCode source-mode editor, aligned with XOOPS BBCode dialect and safely gated on the presence of the vendored SCEditor library.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Documentation
Security