diff --git a/Makefile b/Makefile index 8ea37e8..2e85c8b 100644 --- a/Makefile +++ b/Makefile @@ -399,21 +399,40 @@ up: check-docker @docker exec -u www-data $(DOCKER_NAME) php occ config:system:set apps_paths 1 writable --value=true --type=boolean >/dev/null @echo ">> enabling exelearning" @docker exec -u www-data $(DOCKER_NAME) php occ app:enable exelearning - @# `.elpx` extension → MIME mapping. This is the only piece a - @# Nextcloud app cannot ship by itself: the file is read from + @# `.elp(x)` → MIME mapping + icon alias. These are the only pieces + @# a Nextcloud app cannot ship by itself: both files are read from @# /var/www/html/config/. See README → "Custom MIME types" for the @# manual admin steps required on production installs. - @echo ">> registering .elpx MIME mapping" + @# + @# Both `.elpx` and `.elp` get the same vendor MIME so they share + @# the same eXeLearning icon. `application/zip` stays out of the + @# alias file on purpose — aliasing it would tag every plain ZIP + @# with our icon (the original symptom of issue #21). + @echo ">> registering .elp(x) MIME mapping + icon alias" @docker exec $(DOCKER_NAME) bash -c \ - 'echo "{\"elpx\":[\"application/vnd.exelearning.elpx\",\"application/zip\"]}" \ + 'echo "{\"elpx\":[\"application/vnd.exelearning.elpx\",\"application/zip\"], \"elp\":[\"application/vnd.exelearning.elpx\",\"application/zip\"]}" \ > /var/www/html/config/mimetypemapping.json && \ chown www-data:www-data /var/www/html/config/mimetypemapping.json' + @docker exec $(DOCKER_NAME) bash -c \ + 'echo "{\"application/vnd.exelearning.elpx\":\"exelearning\", \"application/x-exelearning\":\"exelearning\"}" \ + > /var/www/html/config/mimetypealiases.json && \ + chown www-data:www-data /var/www/html/config/mimetypealiases.json' + @# Nextcloud's `maintenance:mimetype:update-js` only scans + @# `core/img/filetypes/` for icon SVGs (see the comment at the top + @# of `core/js/mimetypelist.js`). App-shipped filetype icons are + @# not auto-discovered, so for the dev stack we copy ours into + @# core/ before regenerating the JS list. Production installs need + @# the admin to do the equivalent — see README → "Custom MIME icons". + @echo ">> copying eXeLearning filetype icon into core/img/filetypes" + @docker exec $(DOCKER_NAME) bash -c \ + 'cp /var/www/html/custom_apps/exelearning/img/filetypes/exelearning.svg \ + /var/www/html/core/img/filetypes/exelearning.svg && \ + chown www-data:www-data /var/www/html/core/img/filetypes/exelearning.svg' @docker exec -u www-data $(DOCKER_NAME) php occ maintenance:mimetype:update-js >/dev/null @docker exec -u www-data $(DOCKER_NAME) php occ maintenance:mimetype:update-db --repair-filecache >/dev/null - @# Files-list icons are served by ElpxPreviewProvider via - @# core/preview?...&mimeFallback=true; when an .elpx package has no - @# screenshot.png, the provider returns img/elpx-preview-fallback.png. - @# No additional mimetypealiases.json hack is needed for that. + @# ElpxPreviewProvider still runs for files that actually have a + @# `screenshot.png` inside; for those, the preview overrides the + @# static MIME icon as in upstream Nextcloud. @"$(MAKE)" --no-print-directory seed-fixtures @echo @echo "================================================================" diff --git a/README.md b/README.md index 89e4280..a0b93e6 100644 --- a/README.md +++ b/README.md @@ -218,10 +218,16 @@ admin install should also configure mapping: ```json { - "elpx": ["application/vnd.exelearning.elpx", "application/zip"] + "elpx": ["application/vnd.exelearning.elpx", "application/zip"], + "elp": ["application/vnd.exelearning.elpx", "application/zip"] } ``` +Both extensions get the same vendor MIME so they share the eXeLearning +icon (and the same viewer / editor flow). Legacy `.elp` content is +detected by the editor and migrated to `.elpx` on first save — see +issue #20. + Then refresh Nextcloud's MIME caches: ```bash @@ -231,26 +237,26 @@ sudo -E -u www-data php occ maintenance:mimetype:update-db --repair-filecache Do **not** edit Nextcloud core `mimetypemapping.dist.json` directly. -### Optional: a static `.elpx` MIME icon +### Static `.elp(x)` MIME icon (recommended) The Files list normally shows the preview provided by `ElpxPreviewProvider` (the package's `screenshot.png`, or the bundled -fallback when there is none), so most installs do not need anything -else. If you want `.elpx` files to display a custom icon **outside** -of the preview path — sharing dialogs, breadcrumbs, contexts that -bypass `core/preview` — the documented (admin-side) procedure is: +fallback when there is none). For contexts that bypass `core/preview` +— sharing dialogs, breadcrumbs, the new Vue Files app's icon column — +configure the static MIME icon too: -1. Add an alias to `config/mimetypealiases.json`: +1. Add the alias to `config/mimetypealiases.json`: ```json { "application/vnd.exelearning.elpx": "exelearning", - "application/x-exelearning": "exelearning" + "application/x-exelearning": "exelearning" } ``` 2. Copy this app's icon into Nextcloud core (the only directory - Nextcloud's `MimeIconProvider` scans): + `maintenance:mimetype:update-js` scans for SVGs — see the comment + at the top of `core/js/mimetypelist.js`): ```bash sudo install -o www-data -g www-data -m 0644 \ @@ -266,7 +272,9 @@ bypass `core/preview` — the documented (admin-side) procedure is: ``` Step 2 is brittle because Nextcloud upgrades may replace `core/img/`; -restore it after each upgrade or stage it via a theme override. +restore the SVG after each upgrade or stage it via a theme override. +The dev stack (`make up`) does this automatically — see the +`registering .elp(x) MIME mapping + icon alias` step in the Makefile. ## Viewer integration diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index d0052dd..d7a6019 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -14,8 +14,32 @@ class Application extends App implements IBootstrap { public const APP_ID = 'exelearning'; + /** Modern eXeLearning package MIME, registered for both `.elp` and `.elpx`. */ + public const PRIMARY_MIME_TYPE = 'application/vnd.exelearning.elpx'; + + /** + * MIMEs that unambiguously identify an eXeLearning archive on disk. + * Used by {@see \OCA\ExeLearning\Service\PermissionService::isElpxFile()} + * to decide whether to claim a file by MIME alone — extensions are + * still checked separately. + */ + public const VENDOR_MIME_TYPES = [ + self::PRIMARY_MIME_TYPE, + 'application/x-exelearning', + ]; + + /** + * Broader list of MIMEs an `.elp(x)` file may carry on disk before the + * admin has registered the custom mapping (Nextcloud falls back to + * `application/zip` or `application/octet-stream` in that case). Kept + * for routing decisions where we already know the request is for an + * eXeLearning resource — do **not** use this for "is this an + * eXeLearning file" checks; that path needs `VENDOR_MIME_TYPES` + * combined with an extension check, otherwise every plain ZIP in + * the user's Files would be misclassified. + */ public const ALLOWED_MIME_TYPES = [ - 'application/vnd.exelearning.elpx', + self::PRIMARY_MIME_TYPE, 'application/x-exelearning', 'application/zip', 'application/octet-stream', diff --git a/lib/Preview/ElpxPreviewProvider.php b/lib/Preview/ElpxPreviewProvider.php index 0881a89..5868eda 100644 --- a/lib/Preview/ElpxPreviewProvider.php +++ b/lib/Preview/ElpxPreviewProvider.php @@ -25,8 +25,13 @@ */ class ElpxPreviewProvider implements IProviderV2 { /** - * Regex that matches every MIME alias an .elpx file may carry on disk. - * The double backslash is needed because Nextcloud wraps this in `#…#`. + * Regex that matches every MIME alias an `.elp(x)` file may carry on + * disk. We intentionally cast a wide net here (`zip`, `octet-stream`) + * so Nextcloud asks us about every candidate file; the actual + * "is this really an eXeLearning archive" gate runs in + * {@see isAvailable()} via {@see PermissionService::isElpxFile()}, + * which now requires either a `.elp(x)` extension or a vendor- + * specific MIME (issue #21). */ public const MIME_REGEX = '/^application\/(vnd\.exelearning\.elpx|x-exelearning|zip|octet-stream)$/'; diff --git a/lib/Service/ElpxPackageService.php b/lib/Service/ElpxPackageService.php index 9cd1ab3..b13f743 100644 --- a/lib/Service/ElpxPackageService.php +++ b/lib/Service/ElpxPackageService.php @@ -61,9 +61,15 @@ private function assertAllowed(Node $node): void { if (!$this->permissions->isReadable($node)) { throw new NotPermittedException('No read permission'); } - if (!$this->permissions->isElpxFile($node)) { - throw new NotPermittedException('Not an eXeLearning package'); - } + // We intentionally do NOT gate on `isElpxFile()` here. That check + // is still used by `ElpxPreviewProvider` (so plain ZIPs do not + // inherit our preview), but the viewer / editor flow should + // also accept files the user explicitly opened via the + // "Open as eXeLearning" kebab on a plain `.zip`. The downstream + // validator (`src/elpx/package-validator.ts`) still surfaces a + // clean error for archives that are not actually eXeLearning + // projects, and the user's auth + ownership are already + // guaranteed by `getById()` resolving to a node they can read. if (!$this->permissions->isWithinSizeLimit($node)) { throw new NotPermittedException('Package too large'); } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index eb167b1..b040480 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -32,8 +32,12 @@ public function isElpxFile(Node $node): bool { return true; } } + // Only accept MIME-based matches for genuinely vendor-specific + // types. `application/zip` and `application/octet-stream` would + // drag every plain ZIP / unknown-binary in the user's Files into + // our preview provider, which is the bug behind issue #21. $mime = strtolower((string)$node->getMimeType()); - return in_array($mime, Application::ALLOWED_MIME_TYPES, true); + return in_array($mime, Application::VENDOR_MIME_TYPES, true); } public function isReadable(Node $node): bool { diff --git a/src/files/actions.ts b/src/files/actions.ts index 22330db..2db2a7b 100644 --- a/src/files/actions.ts +++ b/src/files/actions.ts @@ -103,6 +103,36 @@ const editAction: IFileAction = { }, } +// Lets users open a non-.elp(x) zip with the eXeLearning viewer (e.g. +// after `unzip` produced a folder they want to repack and previewed, +// or when a `.elpx` was renamed to `.zip`). Hidden for files we already +// claim by default and for things that aren't zips at all so the menu +// doesn't get crowded. +const openAsExeLearningAction: IFileAction = { + id: 'exelearning-open-as', + displayName: () => t(APP_ID, 'Open as eXeLearning'), + iconSvgInline: () => eyeIconSvgInline(), + enabled: ({ nodes }) => { + if (nodes.length !== 1) return false + const node = nodes[0] as Node + const shape = node as unknown as NodeShape + // Skip if our default action already handles it. + if (isElpxNode(node)) return false + const name = (shape.basename ?? shape.displayName ?? '').toLowerCase() + const mime = (shape.mime ?? '').toLowerCase() + return name.endsWith('.zip') || mime === 'application/zip' + }, + async exec({ nodes }) { + const file = nodes[0] + const fileId = (file as unknown as NodeShape).fileid ?? file.fileid + const url = generateUrl('/apps/exelearning/view?fileId={fileId}', { + fileId: String(fileId ?? ''), + }) + window.open(url, '_self') + return null + }, +} + const downloadAction: IFileAction = { id: 'exelearning-download', displayName: () => t(APP_ID, 'Download .elpx'), @@ -131,4 +161,5 @@ export function registerFileActions(): void { registerFileAction(viewAction) // default — opens /apps/exelearning/view registerFileAction(editAction) // kebab — opens /apps/exelearning/editor registerFileAction(downloadAction) // kebab — native download + registerFileAction(openAsExeLearningAction) // kebab on plain .zip } diff --git a/tests/Unit/AppInfo/ApplicationConstantsTest.php b/tests/Unit/AppInfo/ApplicationConstantsTest.php index 5c4752b..a1e6e83 100644 --- a/tests/Unit/AppInfo/ApplicationConstantsTest.php +++ b/tests/Unit/AppInfo/ApplicationConstantsTest.php @@ -22,4 +22,20 @@ public function testAllowedMimeTypesIncludesLegacyZipFallback(): void { self::assertContains('application/zip', Application::ALLOWED_MIME_TYPES); self::assertContains('application/octet-stream', Application::ALLOWED_MIME_TYPES); } + + /** + * VENDOR_MIME_TYPES is the narrow list used by + * {@see \OCA\ExeLearning\Service\PermissionService::isElpxFile()} to + * decide whether to claim a file by MIME alone. Issue #21 hinged on + * `application/zip` leaking into that path; pin it out here so a + * future edit cannot regress. + */ + public function testVendorMimeTypesDoesNotIncludeGenericArchiveMimes(): void { + self::assertNotContains('application/zip', Application::VENDOR_MIME_TYPES); + self::assertNotContains('application/octet-stream', Application::VENDOR_MIME_TYPES); + } + + public function testVendorMimeTypesIncludesPrimaryMime(): void { + self::assertContains(Application::PRIMARY_MIME_TYPE, Application::VENDOR_MIME_TYPES); + } }