diff --git a/packages/web-app-preview/src/App.vue b/packages/web-app-preview/src/App.vue index 6dbb00206c..7f8eb2596f 100644 --- a/packages/web-app-preview/src/App.vue +++ b/packages/web-app-preview/src/App.vue @@ -124,7 +124,7 @@ import { useImageControls, usePreviewDimensions } from './composables' -import { mimeTypes } from './mimeTypes' +import { mimeTypes, serverRenderedMimeTypes } from './mimeTypes' import { RouteLocationRaw } from 'vue-router' import { SortDir } from '@opencloud-eu/design-system/helpers' @@ -218,7 +218,14 @@ const buildMediaFiles = () => { return false } - return mimeTypes.includes(file.mimeType?.toLowerCase()) && file.canDownload() + const mimeType = file.mimeType?.toLowerCase() + if (!mimeTypes.includes(mimeType) || !file.canDownload()) { + return false + } + + // Keep formats we cannot decode ourselves out of the carousel unless the + // server has a preview for them. + return !serverRenderedMimeTypes.includes(mimeType) || !!file.hasPreview?.() }) const sortFields = determineResourceTableSortFields(filteredFiles[0]) @@ -296,6 +303,11 @@ const loadPreviewImage = async (mediaFile: MediaFile) => { }) } + // `loadPreview` resolves to undefined when the server reports no preview + // for the resource. Formats the browser cannot render on its own (e.g. + // HEIC) then have nothing left to show. Say so instead of leaving an empty + // error frame behind. + mediaFile.isError = !mediaFile.url mediaFile.isLoading = false } catch (e) { if (e.name === 'CanceledError') { diff --git a/packages/web-app-preview/src/index.ts b/packages/web-app-preview/src/index.ts index 7ab58ac7ce..2ed5957611 100644 --- a/packages/web-app-preview/src/index.ts +++ b/packages/web-app-preview/src/index.ts @@ -6,7 +6,7 @@ import { import translations from '../l10n/translations.json' import * as app from './App.vue' import { useGettext } from 'vue3-gettext' -import { mimeTypes } from './mimeTypes' +import { mimeTypes, serverRenderedMimeTypes } from './mimeTypes' import { appId } from './utils' import { extensionPoints } from './extensionPoints' @@ -43,7 +43,8 @@ export default defineWebApplication({ extensions: mimeTypes.map((mimeType) => ({ mimeType, routeName, - label: () => $gettext('Preview') + label: () => $gettext('Preview'), + requiresServerPreview: serverRenderedMimeTypes.includes(mimeType) })) } diff --git a/packages/web-app-preview/src/mimeTypes.ts b/packages/web-app-preview/src/mimeTypes.ts index a944c8f364..21ed3e647f 100644 --- a/packages/web-app-preview/src/mimeTypes.ts +++ b/packages/web-app-preview/src/mimeTypes.ts @@ -5,7 +5,12 @@ export const mimeTypes = [ 'audio/wav', 'audio/x-flac', 'audio/x-wav', + 'image/avif', 'image/gif', + 'image/heic', + 'image/heic-sequence', + 'image/heif', + 'image/heif-sequence', 'image/jpeg', 'image/png', 'image/svg+xml', @@ -17,3 +22,17 @@ export const mimeTypes = [ 'video/quicktime', 'video/webm' ] + +/** + * Subset of the above that the browser cannot decode on its own, so the app can + * only show them if the server renders a preview. Whether it does depends on how + * the thumbnailer was built and on decoders that are not installed everywhere, + * so this is decided per resource via `hasPreview()` rather than up front. + */ +export const serverRenderedMimeTypes = [ + 'image/avif', + 'image/heic', + 'image/heic-sequence', + 'image/heif', + 'image/heif-sequence' +] diff --git a/packages/web-app-preview/tests/unit/app.spec.ts b/packages/web-app-preview/tests/unit/app.spec.ts index 7586878f76..f8d77eb361 100644 --- a/packages/web-app-preview/tests/unit/app.spec.ts +++ b/packages/web-app-preview/tests/unit/app.spec.ts @@ -94,6 +94,26 @@ const activeFiles = [ path: 'personal/admin/labrador.gif', hidden: false, canDownload: () => true + }, + { + id: '10', + fileId: '10', + name: 'otter.heic', + mimeType: 'image/heic', + path: 'personal/admin/otter.heic', + hidden: false, + canDownload: () => true, + hasPreview: () => true + }, + { + id: '11', + fileId: '11', + name: 'badger.heic', + mimeType: 'image/heic', + path: 'personal/admin/badger.heic', + hidden: false, + canDownload: () => true, + hasPreview: () => false } ] @@ -153,6 +173,26 @@ describe('Preview app', () => { expect(mocks.$previewService.loadPreview).not.toHaveBeenCalled() }) + it('marks the media file as failed if the server has no preview to offer', async () => { + const { wrapper, mocks } = createShallowMountWrapper() + await nextTick() + mocks.$previewService.loadPreview.mockClear() + mocks.$previewService.loadPreview.mockResolvedValue(undefined) + + const mediaFile = { + isImage: true, + mimeType: 'image/heic', + isError: false, + isLoading: true, + resource: mock({ isInVault: false, hasPreview: () => false }) + } + await (wrapper.vm as any).loadPreviewImage(mediaFile) + + // nothing to render, and the browser cannot decode HEIC by itself + expect(mediaFile.isError).toBe(true) + expect(mediaFile.isLoading).toBe(false) + }) + it('fetches SVG files via getUrlForResource instead of the preview service', async () => { const { wrapper, mocks, getUrlForResource } = createShallowMountWrapper() await nextTick() @@ -174,7 +214,17 @@ describe('Preview app', () => { describe('Generated "mediaFiles"', () => { it('should hide hidden shares if the share visibility query is not set to "hidden"', () => { const { wrapper } = createShallowMountWrapper() - expect((wrapper.vm as any).mediaFiles.length).toStrictEqual(7) + expect((wrapper.vm as any).mediaFiles.length).toStrictEqual(8) + }) + + it('only includes HEIC files that the server has a preview for', () => { + const { wrapper } = createShallowMountWrapper() + const names = (wrapper.vm as any).mediaFiles.map(({ name }: { name: string }) => name) + + // the browser cannot decode HEIC, so without a server side preview there + // is nothing to page to + expect(names).toContain('otter.heic') + expect(names).not.toContain('badger.heic') }) it('should hide visible shares if the share visibility query is set to "hidden"', async () => { diff --git a/packages/web-pkg/src/apps/types.ts b/packages/web-pkg/src/apps/types.ts index b1c3eb80e2..5364007404 100644 --- a/packages/web-pkg/src/apps/types.ts +++ b/packages/web-pkg/src/apps/types.ts @@ -95,6 +95,15 @@ export interface ApplicationFileExtension { // falls back to "New file.". defaultName?: () => string } + /** + * Set for file types the browser cannot render on its own, so that the app is + * only offered when the server reports a preview for the resource. + * + * Which formats the thumbnailer handles depends on how it was built and on + * decoders that may or may not be installed next to it. Without the flag the + * app would claim such files on every server and open into an error. + */ + requiresServerPreview?: boolean routeName?: string secureView?: boolean } diff --git a/packages/web-pkg/src/composables/actions/files/useFileActions.ts b/packages/web-pkg/src/composables/actions/files/useFileActions.ts index 64201fc632..bd9e921d22 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActions.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActions.ts @@ -125,6 +125,15 @@ export const useFileActions = () => { return false } + // Formats the browser cannot decode itself are only viewable if the + // server renders a preview for them, which depends on the thumbnailer + // build and on optional decoders. Skip the editor action when there is + // none, so the default action (download) can take over instead of + // opening into an error. + if (fileExtension.requiresServerPreview && !resources[0].hasPreview?.()) { + return false + } + // An app may register a file/folder extension purely to // contribute icon mapping or a new-file menu entry without // owning a route (rclone-crypt's vault folder is one such case). diff --git a/packages/web-pkg/src/helpers/resource/icon.ts b/packages/web-pkg/src/helpers/resource/icon.ts index 7a624a07b8..36288cb6be 100644 --- a/packages/web-pkg/src/helpers/resource/icon.ts +++ b/packages/web-pkg/src/helpers/resource/icon.ts @@ -158,10 +158,13 @@ const fileIcon = { icon: { name: 'resource-type-image' }, extensions: [ 'ai', + 'avif', 'cdr', 'eot', 'eps', 'gif', + 'heic', + 'heif', 'jpeg', 'jpg', 'otf', diff --git a/packages/web-pkg/tests/unit/composables/actions/files/useFileActions.spec.ts b/packages/web-pkg/tests/unit/composables/actions/files/useFileActions.spec.ts index 8cda033bbd..dd4d869862 100644 --- a/packages/web-pkg/tests/unit/composables/actions/files/useFileActions.spec.ts +++ b/packages/web-pkg/tests/unit/composables/actions/files/useFileActions.spec.ts @@ -150,6 +150,41 @@ describe('fileActions', () => { }) }) + describe('extensions that need a server rendered preview', () => { + const heicResource = (hasPreview: boolean) => ({ + ...actionOptions, + resources: [ + mock({ + id: '1', + canDownload: () => true, + mimeType: 'image/heic', + extension: 'heic', + hasPreview: () => hasPreview + }) + ] + }) + + it('offers the editor when the server reports a preview', () => { + getWrapper({ + setup: ({ getAllOpenWithActions }) => { + const actions = getAllOpenWithActions(heicResource(true)) + expect(actions.map((a) => a.name)).toContain('editor-preview') + } + }) + }) + + it('hides the editor when the server has no preview to offer', () => { + getWrapper({ + setup: ({ getAllOpenWithActions }) => { + // the download action has to take over instead, opening the editor + // would only show an error + const actions = getAllOpenWithActions(heicResource(false)) + expect(actions.map((a) => a.name)).not.toContain('editor-preview') + } + }) + }) + }) + describe('secure view context', () => { describe('getAllOpenWithActions', () => { it('only displays editors that support secure view', () => { @@ -213,6 +248,12 @@ function getWrapper({ setup }: { setup: (instance: ReturnType