Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nc viewer upgrade to v30.0.0 #16

Merged
merged 18 commits into from
Oct 9, 2024

Conversation

printminion-co
Copy link

printminion-co and others added 18 commits October 8, 2024 18:07
enable via:
./occ config:app:set --value yes --type string viewer always_show_viewer

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
in order later to use it as default viewer

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
in order later to display it for all not known mime types

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Signed-off-by: Misha M.-Kupriyanov <[email protected]>
in order to reuse it later

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
in order to be able to influence it later

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
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 <[email protected]>
Reference the default mimetype from the config module, don't add a magic
string.

Signed-off-by: Thomas Lehmann <[email protected]>
== 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 <[email protected]>
== 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 <[email protected]>
Another state will be added and this prepares for a consistent naming
pattern.t

Signed-off-by: Thomas Lehmann <[email protected]>
== 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 <[email protected]>
For Safari 16.5/167.3 this fixes different icon sizes for the same mime type, visible when cycling through files with different file names.

Signed-off-by: Franziska Bath <[email protected]>
@printminion-co printminion-co changed the title nc viewer upgrade v30.0.0 nc viewer upgrade to v30.0.0 Oct 9, 2024
Copy link

@thlehmann-ionos thlehmann-ionos left a comment

Choose a reason for hiding this comment

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

Looks good to me

@printminion-co printminion-co merged commit a736454 into ionos-dev30 Oct 9, 2024
13 of 20 checks passed
@printminion-co printminion-co deleted the mk/dev/nc-viewer-upgrade-v30.0.0 branch October 9, 2024 09:23
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.

4 participants