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

Fix 'source' URL in viewer #1419

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jkellerer
Copy link

This PR concentrates on filename being consistent and includes app and user so that source URL can be built properly. This allows to use the full viewer (with fileInfo as input) at more places.

Prior to this change, source URL could be invalid and this broke sidebar, download and file-editing in the Viewer. The URL derives from filename but this was not consistently built (sometimes with, sometimes without prefix).

Since the viewer may also need different paths, all input to the viewer is mapped so that it can be adjusted as needed (at a single place).

Note: In my tests albums views still had an issue (e.g. forcing the use of the URL with hasPreview: false fails), but this may be related to nextcloud/viewer#1282

Also ensure 'filename' is consistent and includes app and user

Signed-off-by: Juergen Kellerer <juergen@k123.eu>
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Blocking, I'm investigating a different approach from Viewer for 26, we'll see if that is really necessary to have that heavy changes to fix things :)

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 26, 2022
@vprogramer

This comment was marked as off-topic.

@skjnldsv

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants