feat(styles): administrator-managed style registry for the embedded editor#52
Merged
eXeLearningProject merged 13 commits intomainfrom Apr 29, 2026
Merged
Conversation
…ditor Mirrors the mod_exeweb implementation (PR exelearning/mod_exeweb#32) with the names adjusted for mod_exescorm. Both plugins are kept deliberately parallel: the core logic, admin UX, tests and language strings are identical so fixes to one port trivially to the other. Architecture - `mod_exescorm\local\styles_service` — ZIP validation (path traversal, absolute paths, size cap, extension allow-list, single config.xml), slug allocation with collision suffix, registry persistence via config_plugin(exescorm), and the `build_theme_registry_override()` payload the editor consumes. - `admin/styles.php` — Moodle `admin_externalpage` that renders uploaded/built-in tables and a native `filemanager` (drag-drop, multi-file) for style ZIP upload. On submit the draft area is consumed: each ZIP is validated, extracted into moodledata, and the draft file is deleted. - `editor/styles.php/{slug}/{file}` — PATH_INFO endpoint that serves extracted style assets with capability + registry gating. - `editor/index.php` — injects `window.eXeLearning.config.themeRegistryOverride` and mirrors `blockImportInstall` onto `userStyles` (ONLINE_THEMES_INSTALL) so the 'Import this project style?' modal is suppressed. - `settings.php` — adds link to the styles page (hidden when editor mode ≠ embedded) and the `stylesblockimport` admin checkbox (default: on). Storage Uploaded style bundles extract to `{dataroot}/mod_exescorm/styles/{slug}/`, a sibling of `mod_exescorm/embedded_editor/`, so reinstalling the embedded editor never destroys admin-managed styles. Tests - `tests/local/styles_service_test.php` covers validator edge cases, install, unique-slug on collision, delete, override shape, and the import-blocked config contract. Depends on - exelearning/exelearning#1722 — runtime `themeRegistryOverride` hook (merged). - exelearning/exelearning#1724 — hides the 'Imported' tab when imports are blocked.
Matches upstream eXeLearning ONLINE_THEMES_INSTALL=true, so existing installs and new ones preserve the familiar behavior (imports allowed). Admins now opt-in to the lockdown from Site admin → Styles instead of being opted in silently.
Mirror of exelearning/mod_exeweb refactor. Moves the filemanager upload widget from the standalone admin page into the plugin's own settings page, matching the existing 'Default template' pattern. - classes/admin/admin_setting_stylesupload.php — custom admin_setting_configstoredfile that auto-extracts dropped ZIPs via styles_service::install_from_zip on save. - settings.php — inline upload field + link to the list page; JS toggle hides the whole styles block when editor mode != embedded. - admin/styles.php — list + toggle/delete only; upload moved out. - classes/form/styles_upload_form.php — deleted (superseded). - lang/en + lang/es — new strings and Spanish translations. - tests — stale 'exeweb' string literals replaced with 'exescorm' (sed cleanup of an earlier copy-rename).
Mirror of mod_exeweb refactor: replace the 'Manage installed styles' link with two inline widgets so admins tick/untick styles directly on the plugin settings page, same as any other Moodle multicheckbox setting. - admin_setting_stylesbuiltins: one checkbox per built-in style. - admin_setting_stylesuploaded: one checkbox per uploaded style + inline Delete link. - settings.php: drops the link, wires the two widgets. JS toggle hides the whole styles block (upload, uploaded list, built-in list, block-import) when editor mode != embedded. - lang: new *_hint strings + Spanish translations.
Three integration fixes to bring mod_exescorm in line with wp-exelearning after end-to-end testing in WordPress: - Trap reassignments of window.eXeLearning and window.eXeLearning.config. The static editor's boot sequence reassigns both (the inline script in index.html resets the whole object, and app.bundle.js later parses 'config' from JSON back into an object), so a plain assignment of themeRegistryOverride is wiped before the editor reads it. Wrap the injection in a self-restoring defineProperty getter/setter so the override and the userStyles mirror survive every reset. - Switch the uploaded-style type from 'uploaded' to 'admin' in the override payload. The editor's navbar tabs filter strictly: 'base' | 'site' | 'admin' render in Sistema, 'user' in Imported, and any other value is silently dropped. Admin-uploaded styles match 'admin' semantically and land on the Sistema tab alongside built-ins. - Publish a 'files' manifest alongside each uploaded entry, enumerated from the extracted storage directory. The embedded editor's ResourceFetcher consumes it via themeRegistryOverride.uploaded[].files to fetch admin-approved styles file by file instead of expecting a zip under /bundles/themes/, so preview and HTML5 export pack the real theme assets under theme/* rather than falling back to the placeholder theme/content.css + theme/default.js.
Same linter damage as wp-exelearning: every guard inside is_unsafe_zip_entry was flipping 'return true' to 'return false', so the validator silently accepted path traversal, absolute paths, backslashes, stream schemes and empty entries. Flip the returns back to 'true'.
Three end-to-end bugs discovered after installing a style from the
admin UI and then selecting it in the embedded editor:
- Register the external styles page under the existing 'modsettings'
category. It was added to 'modsettingsexescorm' which does not
exist, so /admin/search.php and the plugin settings loader spammed
"parent does not exist!" debug messages via admin_get_root().
- Read the style drops filearea from component 'exescorm' instead
of 'mod_exescorm'. admin_setting_configstoredfile derives the
component from the first segment of the setting name
('exescorm/styles_drops' → 'exescorm'), so files saved by the
parent's write_setting() live under that component; our
consume_pending_uploads() was walking a filearea that Moodle never
writes to, silently failing to register every uploaded ZIP.
- Require filelib.php before calling send_file() in the styles
serve endpoint. Moodle autoloads lib/setuplib.php, but send_file
lives in lib/filelib.php, so the first request to
/mod/exescorm/editor/styles.php/<slug>/<file> crashed with
"Call to undefined function send_file()" rendered as a themed 404
page, which the embedded editor treated as missing CSS.
ZipArchive::statIndex() surfaces every explicit directory entry in the archive (e.g. 'img/', 'fonts/'), and is_allowed_filename() returns false for any trailing-slash name because it looks for a file extension. Real style packages always contain subdirectories, so uploading a valid ZIP crashed with "stylesupload_badext: img/" and the registry stayed empty. Skip directory entries before the prefix and extension checks — they are not assets, and the per-file extraction path already iterates the full archive and only writes regular files.
ignaciogros
requested changes
Apr 27, 2026
Collaborator
There was a problem hiding this comment.
Thank you. It's a great work. Only a few details are not working correctly:
- When you install the plugin and switch to the inline editor, the block that allows you to install the editor does not appear immediately. After saving, the block does appear. The
onchangeevent is not triggered. The event is trigger if you edit the plugin configuration later. This is not important. It's just a usability issue for admin users. - The
screenshot.pngfor those styles is not displayed. It returns a 404 error:
/mod/exeweb/editor/styles.php/style-name/screenshot.png. - The style I uploaded had icons, but you can't select an icon.
Collaborator
|
If users are not allowed to install a style from a package, maybe this area should be hidden too, as in other versions: 
|
…re/allow-installation-and-management-of-styles
The styles upload setting delegated to admin_setting_configstoredfile, which persists the submitted file path in plugin config and trips its errorsetting validation when the form is saved a second time without changes -- by then consume_pending_uploads() has already extracted and removed the ZIP, so the cached config points at a file that no longer exists. Bypass the parent and stage drafts directly via file_save_draft_area_files(), returning '' regardless of whether a new ZIP was attached. Built-in styles continue to default to enabled because disabled_builtins remains empty on first install.
The static editor's ResourceFetcher rejects on missing CSS / iDevice resources, which surfaces as an "Uncaught (in promise)" that aborts the Yjs theme bind and leaves the editor unresponsive when one of those assets is unreachable (notably under moodle-playground, where static.php cannot resolve every theme path embedded in bundle.json). Port the WP / Omeka-S workaround: wrap window.fetch and jQuery's $.ajaxTransport so 404s on .css / idevices URLs return an empty stylesheet, and short-circuit the editor's preview-sw.js registration so the registration error stops spamming the console without blocking anything. Brings mod_exescorm in line with the other host integrations.
ignaciogros
requested changes
Apr 29, 2026
Collaborator
ignaciogros
left a comment
ignaciogros
left a comment
There was a problem hiding this comment.
Hi @erseco,
I've seen the icons of the installed styles in some cases, but not in others:
I also found that you can't disable an uploaded (an previously enabled) style:
- Go to
/admin/settings.php?section=modsettingexescormand uncheck a style. - Click on "Save".
It's still checked...
Thanks.
admin_find_write_settings() only routes a setting through write_setting() when its parent name appears in $_POST. With every checkbox in the styles_uploaded / styles_builtins widgets cleared the browser submits nothing under that name, so the registry never learned the user disabled the last enabled entry — unchecking the only uploaded style and saving left it enabled. Mirror core's admin_setting_configmulticheckbox template by emitting a hidden sentinel input next to the list so the parent name is always present and write_setting() runs even when nothing is checked.
Collaborator
Author
|
Hi @ignaciogros i can't reproduce "in some cases" please, be more specific. The disable of uploaded themes is fixed, IMHO we should merge this and fix the "some cases" in other specific issue |
ignaciogros
approved these changes
Apr 29, 2026
Collaborator
ignaciogros
left a comment
ignaciogros
left a comment
There was a problem hiding this comment.
Since this PR introduces substantial changes, I think it should be merged into main now, and address those problems in dedicated issues.
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirrors the mod_exeweb PR (exelearning/mod_exeweb#32) with the names
adjusted for mod_exescorm. Both plugins are kept deliberately
parallel: core logic, admin UX, tests, and language strings are
identical so fixes to one port trivially to the other.
Adds a Site administration → Plugins → Activity modules →
eXeLearning (SCORM) → Styles page where managers upload
eXeLearning style
.zippackages, enable/disable built-in styles,and enable/disable/delete uploaded ones — without rebuilding the
static editor bundle.
Changes
classes/local/styles_service.php— pure logic: ZIP validator(traversal / absolute paths / size cap / extension allow-list /
single config.xml), slug allocation with collision suffix, registry
persistence via
config_plugin(exescorm), and thebuild_theme_registry_override()payload the editor consumes.classes/form/styles_upload_form.php— Moodlemoodleformwith anative
filemanager(drag-and-drop, multi-file, native filetyperestrictions).
admin/styles.php—admin_externalpagethat consumes the draftfilearea after upload, validates and extracts each ZIP, and reports
a per-file summary.
editor/styles.php/{slug}/{file}— PATH_INFO endpoint that servesextracted style assets from moodledata with capability + registry
gating.
editor/index.php— injectswindow.eXeLearning.config.themeRegistryOverrideand mirrorsblockImportInstallonto the pre-existinguserStyles(ONLINE_THEMES_INSTALL) flag so the "Import this project style?"
modal is suppressed end-to-end.
settings.php— link to the styles page (hidden when editor mode≠ embedded) and the
stylesblockimportadmin checkbox (default: on).lang/en/exescorm.php— newstyles*strings.tests/local/styles_service_test.php— unit tests.Storage
Uploaded style bundles extract to
{dataroot}/mod_exescorm/styles/{slug}/, a sibling ofmod_exescorm/embedded_editor/, so reinstalling the embedded editornever destroys admin-managed styles.
Depends on
themeRegistryOverridehook (merged).Test plan
php -lpasses on all new/modified files.suffix, delete, override shape, import-blocked contract.
confirm they show in the editor's selector.
editor's "Imported" tab appears/disappears accordingly.
editor falls back to
basewithout errors.Moodle Playground Preview
The changes in this pull request can be previewed and tested using a Moodle Playground instance.