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 aliasing mimes #1456

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

Fix aliasing mimes #1456

wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 9, 2022

Fix regression by #1273
Fix #1450

@skjnldsv skjnldsv self-assigned this Nov 9, 2022
@skjnldsv skjnldsv added bug Something isn't working regression Regression of a previous working feature 3. to review Waiting for reviews labels Nov 9, 2022
@skjnldsv
Copy link
Member Author

skjnldsv commented Nov 9, 2022

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Nov 9, 2022
Signed-off-by: John Molakvoæ <[email protected]>
@@ -595,7 +595,7 @@ export default {
fileInfo = this.fileList[this.currentIndex]

// show file
this.currentFile = new File(fileInfo, mime, handler.component)
this.currentFile = new File(fileInfo, mime, this.components[mime])
Copy link
Member Author

@skjnldsv skjnldsv Nov 9, 2022

Choose a reason for hiding this comment

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

If we use the original handler instead of the registered aliases, we break all aliases on file opening.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could check again if the override handler is specified, if so we can use the original handler coponent?

const component = overrideHandlerId ? handler.component : this.components[mime];
this.currentFile = new File(fileInfo, mime, component)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we should be consistent on how we do things for other File creations :)

this.currentFile = new File(fileInfo, mime, this.components[mime])

this.previousFile = new File(prev, mime, this.components[mime])

this.nextFile = new File(next, mime, this.components[mime])

@jkhsjdhjs
Copy link

Can confirm, this fixes the issue on my nextcloud 25.0.1 instance.

@Raudius
Copy link
Contributor

Raudius commented Nov 9, 2022

Doesn't this revert the behaviour back to always use the viewer compontent based on mime type? This would then break the openWith method.

@skjnldsv skjnldsv added this to the Nextcloud 26 milestone Feb 25, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@skjnldsv skjnldsv modified the milestones: Nextcloud 27, Nextcloud 28 May 3, 2023
@skjnldsv skjnldsv marked this pull request as draft May 11, 2023 07:10
@ariselseng
Copy link
Member

It would be nice if this can be worked on. My app camerarawpreviews has quite a few users, and currently does not work with Viewer. I know it did before though.
Here is my use of this feature: https://github.com/ariselseng/camerarawpreviews/blob/master/js/register-viewer.js
If you would do a rebase @skjnldsv I can certainly try it out locally and see if it helps.

@joshtrichards
Copy link
Member

@ariselseng Right now this PR is set in draft mode so it's probably being overlooked for formal reviews.

Seems to have some positive feedback from testers, but also an outstanding question from @Raudius. I'm not familiar with this area of code so I don't feel qualified commenting one way or another.

@skjnldsv: This is your PR - thoughts? Okay to pull this out of draft? Need us find some more folks to test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress backport-request Pending backport by the backport-bot bug Something isn't working regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RAW photos are not displayed with infinite spinner
7 participants