Conversation
92e753a to
c0f841d
Compare
8018718 to
7a2fb77
Compare
88fcb71 to
59fecf7
Compare
93ec687 to
bc519a9
Compare
3aa0273 to
82d2afc
Compare
|
Ready for reviews! 💪 🚀 |
4b826c1 to
bea8c51
Compare
susnux
left a comment
There was a problem hiding this comment.
Code can already reviewed, but blocking merge for now, will discuss next week.
7212571 to
597d783
Compare
Replace the legacy OCA.Viewer.registerHandler() global with registerHandler() from @nextcloud/viewer, following the removal of the bundled viewer app in favour of the package (nextcloud/server#63954). Key changes: - viewer.js: register a custom element (oca-text-viewer) via defineCustomElement and call registerHandler() from @nextcloud/viewer - TextViewerWrapper.vue: thin wrapper mapping new IFile-based ViewerProps to the existing ViewerComponent props - LoadViewerListener: switch from OCA\Viewer\Event\LoadViewer (gone with the viewer app) to OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent, loading text-viewer as an init script on every non-error page - Application.php: update event listener registration accordingly - Add @nextcloud/viewer ^2.0.0-beta.11 dependency canCompare and downloadCallback are not part of the new API and have been dropped. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Replace the legacy OCA.Viewer.registerHandler() global with registerHandler() from @nextcloud/viewer, following the removal of the bundled viewer app in favour of the package (nextcloud/server#63954). Key changes: - viewer.js: register a custom element (oca-text-viewer) via defineCustomElement and call registerHandler() from @nextcloud/viewer - TextViewerWrapper.vue: thin wrapper mapping new IFile-based ViewerProps to the existing ViewerComponent props - LoadViewerListener: switch from OCA\Viewer\Event\LoadViewer (gone with the viewer app) to OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent, loading text-viewer as an init script on every non-error page - Application.php: update event listener registration accordingly - Add @nextcloud/viewer ^2.0.0-beta.11 dependency canCompare and downloadCallback are not part of the new API and have been dropped. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Replace the legacy OCA.Viewer.registerHandler() global with registerHandler() from @nextcloud/viewer, following the removal of the bundled viewer app in favour of the package (nextcloud/server#63954). Key changes: - viewer.js: register a custom element (oca-text-viewer) via defineCustomElement and call registerHandler() from @nextcloud/viewer - TextViewerWrapper.vue: thin wrapper mapping new IFile-based ViewerProps to the existing ViewerComponent props - LoadViewerListener: switch from OCA\Viewer\Event\LoadViewer (gone with the viewer app) to OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent, loading text-viewer as an init script on every non-error page - Application.php: update event listener registration accordingly - Add @nextcloud/viewer ^2.0.0-beta.11 dependency canCompare and downloadCallback are not part of the new API and have been dropped. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
The suite the viewer app carried in Cypress, ported to Playwright and to the fixtures the server harness already sets up: opening every media type, navigating between files, sorting, sharing, odd names, and the actions around them. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
2.0.0-beta.9, which carries the viewer and its handlers. The entry is built with the Vue 3 frontend rather than the legacy one. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
The viewer only offers a mime its handler can actually show, and for HEIC, HEIF, TIFF and friends that means the server has to render a preview. The app read the enabled providers from an initial state it provided itself; a package cannot, so core reports them under capabilities.previews.enabled_providers, the same array_keys() the app used. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
A file link is not only on the Files list: an app can put one on any page, and the old LoadViewer event existed so those pages could ask the app to load itself. Core puts the init script on every rendered page instead, the themed error page aside, and the script only registers the handlers: nothing of the viewer is fetched until a file is opened. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
The global was the app's own API and goes with it. The versions sidebar, the local-edit action and the settings PDF link ask the package instead. Comparison stays with the pictures it means something for: the old viewer asked each handler through a canCompare flag that none of the bundled ones set, and the package carries no such flag yet. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Nothing needs it any more: the handlers come from @nextcloud/viewer, core puts the init script on every page, and the enabled preview providers are a core capability. With it goes the LoadViewer event, which three places dispatched behind a class_exists() guard to ask the app to load itself. An app that wants a file view imports the package and registers a handler; it no longer matters whether the viewer app is enabled, or which app got there first. The app config rows left behind on an upgrade are cleaned up by the repair step that already does this for the other apps we dropped. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Three of them asked a question whose answer changes under them: waitForClosed asked whether the first matching modal was hidden, which passes while a second one is still up. It counts them now, and close() waits for openfile to leave the URL before looking. runAction decided between a header button and a menu entry with one isVisible() call. The header re-renders whenever the shown file changes, so on the second delete it found neither and then waited for a menu it had not opened. The sequence is retried as a unit. The sidebar test asserted the loading spinner was on screen. The list fetches the preview for its own row and the viewer reuses it, so delaying the viewer's request holds nothing back: the spinner measured about 600ms, which passed on CI and failed here. It asserts the action it exists for instead. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
The old suite had two specs for it and the port dropped both: a viewer opened on a list of its own rather than a folder, which is the API Photos and the versions sidebar use, and loadMore continuing that list when navigation runs out. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Dropping the bundled app took the OCA.Viewer global with it, and apps in the wild still call it: of the ones in tree, text and recommendations ask it what it can open, photos opens and closes it, and files_pdfviewer re-opens the current file with another handler. So it goes back on the page, backed by the package rather than by the app. open() takes the paths and the loose file info apps passed before and turns them into nodes, the getters answer from what the last open() was given, and mimetypes answers by asking the handlers about a file of that type, since a handler no longer carries a list of them. registerHandler is deliberately not here. It was deprecated in the viewer app in August 2025 and this is where it stops. Every use logs a warning naming both the replacement and Nextcloud 40, which is where this file gets deleted. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
536e3bf to
9de1d27
Compare
There was a problem hiding this comment.
But we still need the viewer to be available to provide OCA\Viewer\Event\LoadViewer as this is checked on apps in the backend, no?
There was a problem hiding this comment.
No, they can remove it entirely.
They need to provide their script on every page with the BeforeTemplateRenderedEvent 👍
There was a problem hiding this comment.
I am talking about apps that want to use the viewer to show files. They must keep working with Nextcloud 36 without any adjustments on the app side.
There was a problem hiding this comment.
So we'll have to keep a useless class?
Sure, why not 😅
viewerapp into server repository #60480Summary
The viewer used to be a bundled app that every other app had to check for before
it could register a handler. It is a library now,
@nextcloud/viewerinnextcloud-libraries/nextcloud-viewer,
and this drops the app.
An app that wants to add a file view imports the package and registers its
handler. It no longer matters whether the viewer app is enabled, or which app
loaded it first: several copies on a page elect the newest between them, and only
that copy fetches the viewer, only when a file is opened.
What is left in the server
apps/filesputs a thirteen line init script on every page a file can be openedfrom, on the same event the viewer app's own listener used. The script does
nothing but import the package; the handlers for images, video and audio come
with it.
dist/files-viewer-init.jsis 24 kB, 8.3 kB gzipped.LoadViewergoes with the app. Three places dispatched it behind aclass_exists()guard to ask the app to load itself; none of them need to now.enabled_preview_providers becomes a core capability
Which mimes have a preview provider decides what the viewer offers to open: heic,
heif and tiff are viewable through a preview and not otherwise. That was an
initial state the viewer app provided, so anything wanting it had to be on a page
that app had touched. It is
core.previews.enabled_providersnow, next to thecapabilities clients already read, public share pages included.
PreviewManager::getProviders()registers closures and instantiates no provider,so this costs nothing per request, and it reports nothing when previews are off.
OCA.Viewer is deprecated, and three callers were still on it
Viewer 7.0.0 dropped the global. These do nothing on current master:
They take nodes now. The versions sidebar builds one for the version it shows,
pointing at the version's own dav endpoint rather than at the 250px thumbnail
meant for the list;
canView()from the package decides whether to offer thebutton, which is the same test the click-to-open action uses.
Other apps have to move to the package, tracked in
nextcloud-libraries/nextcloud-viewer#17. So that one not ported in time does not
break outright,
core/src/viewer-legacy.tskeepsopen,openWith,compare,closeand the getters working on top of the package, with a deprecation warningon every access, until 40.
registerHandleris not kept, so text (nextcloud/text#9235)and files_pdfviewer (nextcloud/files_pdfviewer#1572) stop opening their files until
they are ported.
Testing
tests/playwright/e2e/viewer/, which comes over from the viewer's ownrepository in the first commit: 46 passed, 1 skipped
versionToNodeand the capabilitytests/lib/OCS/CoreCapabilitiesTest.php,tests/lib/AppTest.php,tests/lib/App/AppManagerTest.phpUpgrades:
viewerjoins the abandoned appsCleanUpAbandonedAppsalreadycleans the config rows for.
TODO
OCA.ViewerAPI and theLoadViewereventChecklist
3. to review, feature component)stable32)AI (if applicable)