Skip to content

feat(styles): administrator-managed style registry for the embedded editor#52

Open
erseco wants to merge 8 commits intomainfrom
feature/allow-installation-and-management-of-styles
Open

feat(styles): administrator-managed style registry for the embedded editor#52
erseco wants to merge 8 commits intomainfrom
feature/allow-installation-and-management-of-styles

Conversation

@erseco
Copy link
Copy Markdown
Collaborator

@erseco erseco commented Apr 23, 2026

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 .zip packages, 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 the
    build_theme_registry_override() payload the editor consumes.
  • classes/form/styles_upload_form.php — Moodle moodleform with a
    native filemanager (drag-and-drop, multi-file, native filetype
    restrictions).
  • admin/styles.phpadmin_externalpage that consumes the draft
    filearea after upload, validates and extracts each ZIP, and reports
    a per-file summary.
  • editor/styles.php/{slug}/{file} — PATH_INFO endpoint that serves
    extracted style assets from moodledata with capability + registry
    gating.
  • editor/index.php — injects
    window.eXeLearning.config.themeRegistryOverride and mirrors
    blockImportInstall onto the pre-existing userStyles
    (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 stylesblockimport admin checkbox (default: on).
  • lang/en/exescorm.php — new styles* strings.
  • tests/local/styles_service_test.php — unit tests.

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.

Depends on

Test plan

  • php -l passes on all new/modified files.
  • Unit test suite covers validator, registry, install, collision
    suffix, delete, override shape, import-blocked contract.
  • Upload one or multiple style .zips via the filemanager and
    confirm they show in the editor's selector.
  • Toggle "Block user-imported styles" on/off; confirm the
    editor's "Imported" tab appears/disappears accordingly.
  • Open an .elpx that references a missing style; confirm the
    editor falls back to base without errors.

Moodle Playground Preview

The changes in this pull request can be previewed and tested using a Moodle Playground instance.

Preview in Moodle Playground

⚠️ The embedded eXeLearning editor is not included in this preview. You can install it from Modules > eXeLearning SCORM > Configure using the "Download & Install Editor" button. All other module features (ELPX upload, viewer, preview) work normally.

erseco added 8 commits April 23, 2026 19:22
…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.
Copy link
Copy Markdown
Collaborator

@ignaciogros ignaciogros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 onchange event 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.png for 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.
imagen

@ignaciogros
Copy link
Copy Markdown
Collaborator

If users are not allowed to install a style from a package, maybe this area should be hidden too, as in other versions:

imagen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants