From b372b2e6bddd90689edd58a2eadd40c09efe7cc2 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 27 Aug 2024 12:48:43 +0200 Subject: [PATCH 01/20] feat: add always_show_viewer app config value enable via: ./occ config:app:set --value yes --type string viewer always_show_viewer Signed-off-by: Misha M.-Kupriyanov --- lib/Listener/LoadViewerScript.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Listener/LoadViewerScript.php b/lib/Listener/LoadViewerScript.php index c59f71b88..10ab868b3 100644 --- a/lib/Listener/LoadViewerScript.php +++ b/lib/Listener/LoadViewerScript.php @@ -28,6 +28,7 @@ use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Viewer\AppInfo\Application; use OCA\Viewer\Event\LoadViewer; +use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -40,13 +41,16 @@ class LoadViewerScript implements IEventListener { private IInitialState $initialStateService; private IPreview $previewManager; + private IAppConfig $appConfig; public function __construct( IInitialState $initialStateService, - IPreview $previewManager + IPreview $previewManager, + IAppConfig $appConfig ) { $this->initialStateService = $initialStateService; $this->previewManager = $previewManager; + $this->appConfig = $appConfig; } public function handle(Event $event): void { @@ -54,8 +58,11 @@ public function handle(Event $event): void { return; } + $alwaysShowViewer = $this->appConfig->getAppValue('always_show_viewer', 'no') === 'yes'; + Util::addStyle(Application::APP_ID, 'viewer-main'); Util::addScript(Application::APP_ID, 'viewer-main', 'files'); $this->initialStateService->provideInitialState('enabled_preview_providers', array_keys($this->previewManager->getProviders())); + $this->initialStateService->provideInitialState("always_show_viewer", $alwaysShowViewer); } } From df5884c4dfa9b742ff278d048fdc597a30d15455 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 27 Aug 2024 17:05:56 +0200 Subject: [PATCH 02/20] feat: add config module to expose alwaysShowViewer Signed-off-by: Misha M.-Kupriyanov --- src/models/config.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/models/config.ts diff --git a/src/models/config.ts b/src/models/config.ts new file mode 100644 index 000000000..cbe1653fd --- /dev/null +++ b/src/models/config.ts @@ -0,0 +1,12 @@ +/** + * SPDX-FileLicenseText: 2024 STRATO AG + * SPDX-License-Identifier: AGPL-3.0-or-later + * SPDX-FileContributor: Mikhailo Matiyenko-Kupriyanov + */ +import { loadState } from '@nextcloud/initial-state' + +const alwaysShowViewer = loadState('viewer', 'always_show_viewer', false) + +export default { + alwaysShowViewer, +} From d2fba277506bb7214d778ad8c160643af10fb1f0 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 27 Aug 2024 17:16:53 +0200 Subject: [PATCH 03/20] feat: add default component stub in order later to use it as default viewer Signed-off-by: Misha M.-Kupriyanov --- src/components/Default.vue | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/components/Default.vue diff --git a/src/components/Default.vue b/src/components/Default.vue new file mode 100644 index 000000000..e9a47575b --- /dev/null +++ b/src/components/Default.vue @@ -0,0 +1,22 @@ + + + + + + From d6c3bf144d95553541d8dc3f5c27d4796c404e1a Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 28 Aug 2024 12:50:49 +0200 Subject: [PATCH 04/20] feat: add default viewer in order later to display it for all not known mime types Signed-off-by: Misha M.-Kupriyanov --- src/models/default.ts | 16 ++++++++++++++++ src/services/Viewer.js | 2 ++ 2 files changed, 18 insertions(+) create mode 100644 src/models/default.ts diff --git a/src/models/default.ts b/src/models/default.ts new file mode 100644 index 000000000..9de86a667 --- /dev/null +++ b/src/models/default.ts @@ -0,0 +1,16 @@ +/** + * SPDX-FileCopyrightText: 2024 STRATO AG + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import Default from '../components/Default.vue' + +export default { + id: 'default', + group: 'other', + mimes: [ + '*/*', + ], + mimesAliases: {}, + component: Default, +} diff --git a/src/services/Viewer.js b/src/services/Viewer.js index 3636293c3..89fb4bb08 100644 --- a/src/services/Viewer.js +++ b/src/services/Viewer.js @@ -23,6 +23,7 @@ import Images from '../models/images.js' import Videos from '../models/videos.js' import Audios from '../models/audios.js' +import Default from '../models/default.ts' /** * Handler type definition @@ -77,6 +78,7 @@ export default class Viewer { this.registerHandler(Images) this.registerHandler(Videos) this.registerHandler(Audios) + this.registerHandler(Default) console.debug('OCA.Viewer initialized') } From 67dd1597941dbb1e5f1ca16d2d5ebe48e463c3ca Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 28 Aug 2024 12:54:29 +0200 Subject: [PATCH 05/20] feat: enable default viewer Signed-off-by: Misha M.-Kupriyanov --- src/views/Viewer.vue | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index 83616c8f5..e9de8a8ba 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -204,6 +204,7 @@ import isMobile from '@nextcloud/vue/dist/Mixins/isMobile.js' import { extractFilePaths, sortCompare } from '../utils/fileUtils.ts' import canDownload from '../utils/canDownload.js' import cancelableRequest from '../utils/CancelableRequest.js' +import configModule from '../models/config.ts' import Error from '../components/Error.vue' import File from '../models/file.js' import filesActionHandler from '../services/FilesActionHandler.js' @@ -682,6 +683,11 @@ export default { handler = this.registeredHandlers[mime] ?? this.registeredHandlers[alias] } + // fallback to default viewer if enabled + if (!handler && configModule.alwaysShowViewer) { + handler = this.registeredHandlers['*/*'] + } + // if we don't have a handler for this mime, abort if (!handler) { logger.error('The following file could not be displayed', { fileInfo }) @@ -953,6 +959,12 @@ export default { if (nodes.some(node => !(node.isDavRessource && node.root?.startsWith('/files')))) { return false } + + // Always enabled if configured so + if (configModule.alwaysShowViewer) { + return true + } + // Faster to check if at least one node doesn't match the requirements return !nodes.some(node => ( (node.permissions & Permission.READ) === 0 From e344acebab672cf7c8b61870aee84311fbfc6f3f Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 3 Sep 2024 14:28:53 +0200 Subject: [PATCH 06/20] feat: extract default mime type to config in order to reuse it later Signed-off-by: Misha M.-Kupriyanov --- src/models/config.ts | 1 + src/views/Viewer.vue | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/models/config.ts b/src/models/config.ts index cbe1653fd..989d1f719 100644 --- a/src/models/config.ts +++ b/src/models/config.ts @@ -9,4 +9,5 @@ const alwaysShowViewer = loadState('viewer', 'always_show_viewer', fals export default { alwaysShowViewer, + defaultMimeType: '*/*', } diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index e9de8a8ba..97c2b5fee 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -685,7 +685,7 @@ export default { // fallback to default viewer if enabled if (!handler && configModule.alwaysShowViewer) { - handler = this.registeredHandlers['*/*'] + handler = this.registeredHandlers[configModule.defaultMimeType] } // if we don't have a handler for this mime, abort From a720b92595a8fb3761ac34df898dce3dce676123 Mon Sep 17 00:00:00 2001 From: Franziska Bath Date: Mon, 2 Sep 2024 13:37:47 +0200 Subject: [PATCH 07/20] feat: properly implement default component Signed-off-by: Franziska Bath --- src/components/Default.vue | 48 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/components/Default.vue b/src/components/Default.vue index e9a47575b..f02c5a753 100644 --- a/src/components/Default.vue +++ b/src/components/Default.vue @@ -5,18 +5,62 @@ --> From 63febbe17fbe9cd72beb331732f33751b250f02b Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 3 Sep 2024 13:26:55 +0200 Subject: [PATCH 08/20] feat(Viewer): extract modal title as own method in order to be able to influence it later Signed-off-by: Misha M.-Kupriyanov --- src/views/Viewer.vue | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index 97c2b5fee..b46d9a7a4 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -60,7 +60,7 @@ :inline-actions="canEdit ? 1 : 0" :spread-navigation="true" :style="{ width: isSidebarShown ? `${sidebarPosition}px` : null }" - :name="currentFile.basename" + :name="modalTitle" :view="currentFile.modal" class="viewer" size="full" @@ -412,6 +412,10 @@ export default { } }, + modalTitle() { + return this.currentFile.basename + }, + showComparison() { return !this.isMobile }, From 94d6c47f69b3f8c5f2d3c6387eb4a41dc9bbf84f Mon Sep 17 00:00:00 2001 From: Franziska Bath Date: Mon, 2 Sep 2024 13:41:26 +0200 Subject: [PATCH 09/20] feat(Viewer): ensure previous and next navigation works with default component Signed-off-by: Franziska Bath --- src/views/Viewer.vue | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index b46d9a7a4..62756ab12 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -703,8 +703,10 @@ export default { this.theme = handler.theme ?? 'dark' this.handlerId = handler.id + // fallback to default viewer group if enabled + const groupFallback = configModule.alwaysShowViewer ? this.mimeGroups[configModule.defaultMimeType] : undefined // check if part of a group, if so retrieve full files list - const group = this.mimeGroups[mime] + const group = this.mimeGroups[mime] ?? groupFallback if (this.files && this.files.length > 0) { logger.debug('A files list have been provided. No folder content will be fetched.') // we won't sort files here, let's use the order the array has @@ -723,8 +725,8 @@ export default { const [dirPath] = extractFilePaths(fileInfo.filename) const fileList = await folderRequest(dirPath) - // filter out the unwanted mimes - const filteredFiles = fileList.filter(file => file.mime && mimes.indexOf(file.mime) !== -1) + // filter out the unwanted mimes if configModule.alwaysShowViewer not enabled + const filteredFiles = configModule.alwaysShowViewer ? fileList : fileList.filter(file => file.mime && mimes.indexOf(file.mime) !== -1) // sort like the files list // TODO: implement global sorting API @@ -758,7 +760,7 @@ export default { openFileFromList(fileInfo) { // override mimetype if existing alias const mime = fileInfo.mime - this.currentFile = new File(fileInfo, mime, this.components[mime]) + this.currentFile = new File(fileInfo, mime, this.components[mime] || this.components[configModule.defaultMimeType]) this.changeSidebar() this.updatePreviousNext() }, From 2cb5bff2e934ee06ee3a38f09655df8221813f55 Mon Sep 17 00:00:00 2001 From: Franziska Bath Date: Mon, 2 Sep 2024 13:40:22 +0200 Subject: [PATCH 10/20] feat(Viewer): hide file name in modal header when mime image shown Signed-off-by: Franziska Bath --- src/views/Viewer.vue | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index 62756ab12..adbd1eaaa 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -413,7 +413,11 @@ export default { }, modalTitle() { - return this.currentFile.basename + if (!configModule.alwaysShowViewer) { + return this.currentFile.basename + } + + return this.currentFile?.modal?.name === 'Default' ? '' : this.currentFile.basename }, showComparison() { From 889df8b6e1053e11d96e8dd96484ba4974e30090 Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Wed, 11 Sep 2024 17:50:37 +0200 Subject: [PATCH 11/20] fix(Viewer): remove directories from fileList Don't include directories as they can not be displayed. Note: including directories could also cause a follow-up error with certain directory structures which happen to include a directory named like a number (i.e. 123) because of sloppy, too broad type casting in fileUtils.ts's genFileInfo() accidentally converting such a folder name to a Number, which then can not be used in string comparisons. Signed-off-by: Thomas Lehmann --- src/views/Viewer.vue | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index adbd1eaaa..a56330b58 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -729,8 +729,14 @@ export default { const [dirPath] = extractFilePaths(fileInfo.filename) const fileList = await folderRequest(dirPath) - // filter out the unwanted mimes if configModule.alwaysShowViewer not enabled - const filteredFiles = configModule.alwaysShowViewer ? fileList : fileList.filter(file => file.mime && mimes.indexOf(file.mime) !== -1) + let filteredFiles + if (configModule.alwaysShowViewer) { + // don't include directories, otherwise accept all mimes + filteredFiles = fileList.filter(({ type }) => type !== 'directory') + } else { + // filter out the unwanted mimes + filteredFiles = fileList.filter(file => file.mime && mimes.indexOf(file.mime) !== -1) + } // sort like the files list // TODO: implement global sorting API From 405d4e44610d6051b2b3b06df2587c98030acb97 Mon Sep 17 00:00:00 2001 From: Kai Henseler Date: Thu, 12 Sep 2024 12:19:32 +0200 Subject: [PATCH 12/20] fix(Viewer): remove contextmenu view action for folders Signed-off-by: Kai Henseler --- src/views/Viewer.vue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index a56330b58..b89cc74ee 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -978,6 +978,10 @@ export default { // Always enabled if configured so if (configModule.alwaysShowViewer) { + // disable for folders + if (nodes.some(node => node.type === 'folder')) { + return false + } return true } From 02833aed2213d789dd230ddcb981ba0411ed3f7a Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Thu, 12 Sep 2024 13:58:00 +0200 Subject: [PATCH 13/20] refactor: use config module, not magic string Reference the default mimetype from the config module, don't add a magic string. Signed-off-by: Thomas Lehmann --- src/models/default.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/models/default.ts b/src/models/default.ts index 9de86a667..94ef98f3a 100644 --- a/src/models/default.ts +++ b/src/models/default.ts @@ -4,12 +4,13 @@ */ import Default from '../components/Default.vue' +import config from './config.ts' export default { id: 'default', group: 'other', mimes: [ - '*/*', + config.defaultMimeType, ], mimesAliases: {}, component: Default, From ea97d0866c289c4868829775510b527ea4f43cd5 Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Thu, 12 Sep 2024 13:58:57 +0200 Subject: [PATCH 14/20] fix: change default mimetype */* -> "all" == The bug The app config "always_show_viewer" enables the preview for all mimetypes. If this config is not set and no handler is registered for a mimetype, the file will be downloaded. In the share/public view, with this config enabled some file types were downloaded instead of opened in the preview. The code would not progress up to Viewer's openFileInfo() because it would not find a preview component candidate in [1] to even attempt opening the preview. == The fix As per reverse engineering it was found that special string "all" is used as symbol for handling any mimetype (at least in [2]). The decision was made to change the special mimetype for a registered previewer to "all", because then handling any file is already coverered this way. All discovered places: 1. Files/fileactions: getDefaultFileAction() [2] 2. Shares: attach(), "fileActionsReady" event handler, registerAction() call [3] [1]: https://github.com/nextcloud/server/blob/v29.0.6/apps/files/js/filelist.js#L912 [2]: https://github.com/nextcloud/server/blob/v29.0.6/apps/files/js/fileactions.js#L315 [3]: https://github.com/nextcloud/server/blob/v29.0.6/apps/files_sharing/src/share.js#L230 Signed-off-by: Thomas Lehmann --- src/models/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/config.ts b/src/models/config.ts index 989d1f719..ca6096b6b 100644 --- a/src/models/config.ts +++ b/src/models/config.ts @@ -9,5 +9,5 @@ const alwaysShowViewer = loadState('viewer', 'always_show_viewer', fals export default { alwaysShowViewer, - defaultMimeType: '*/*', + defaultMimeType: 'all', } From 21475a1d03a000a8631ed0fb37170d5eb290242a Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Thu, 12 Sep 2024 17:19:55 +0200 Subject: [PATCH 15/20] fix(Default): define proper text color == The error The text below the mimetype icon has Nextcloud's default text styling, which is dark text on light background, yet here it's dark background. == The fix The default component now defines a text color. Signed-off-by: Thomas Lehmann --- src/components/Default.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Default.vue b/src/components/Default.vue index f02c5a753..5cf19ff5e 100644 --- a/src/components/Default.vue +++ b/src/components/Default.vue @@ -57,6 +57,7 @@ img { } .title { + color: var(--color-primary-element-text); font-weight: bold; font-size: 1.4em; overflow: hidden; From 0b8be33a561b171397581506dff9be5e81d7c134 Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Thu, 12 Sep 2024 15:45:24 +0200 Subject: [PATCH 16/20] refactor(Images): rename preview load failed state variable Another state will be added and this prepares for a consistent naming pattern.t Signed-off-by: Thomas Lehmann --- src/components/Images.vue | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Images.vue b/src/components/Images.vue index c8a8ce257..12eb9b0e7 100644 --- a/src/components/Images.vue +++ b/src/components/Images.vue @@ -126,7 +126,7 @@ export default { shiftX: 0, shiftY: 0, zoomRatio: 1, - fallback: false, + previewFailed: false, livePhotoCanBePlayed: false, } }, @@ -195,7 +195,7 @@ export default { } // If loading the preview failed once, let's load the original file - if (this.fallback) { + if (this.previewFailed) { return this.src } @@ -355,9 +355,9 @@ export default { // Fallback to the original image if not already done onFail() { - if (!this.fallback) { + if (!this.previewFailed) { console.error(`Loading of file preview ${basename(this.src)} failed, falling back to original file`) - this.fallback = true + this.previewFailed = true } }, doneLoadingLivePhoto() { From 00fb66286f9be0da4a9381d59c6da588a552e1da Mon Sep 17 00:00:00 2001 From: Thomas Lehmann Date: Thu, 12 Sep 2024 15:41:35 +0200 Subject: [PATCH 17/20] fix(Images): hide loading spinner on failed image load == The cause Previously the code attempted to load a preview of an image. If loading this preview image failed it was attempted to load the original image. Load errors of images were only handled _once_. This meant that a load error for the original image was never handled, thus the viewer was still in loading state and showed a browser-dependant "broken image" replacement icon. == The fix Now further image load errors are handled too. In case the original fails too, the loading state is ended and a placeholder text is shown. The default preview component, which was introduced to show something for any mimetype if configured, is now also used as a fallback. Signed-off-by: Thomas Lehmann --- src/components/Images.vue | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/components/Images.vue b/src/components/Images.vue index 12eb9b0e7..188e0b1c7 100644 --- a/src/components/Images.vue +++ b/src/components/Images.vue @@ -29,7 +29,8 @@ @close="onClose" />