Skip to content

Comments

Fix aliasing mimes#1456

Closed
skjnldsv wants to merge 1 commit intomasterfrom
fix/aliases
Closed

Fix aliasing mimes#1456
skjnldsv wants to merge 1 commit intomasterfrom
fix/aliases

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 9, 2022 •
edited
Loading

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
@skjnldsv
Fix aliasing mimes
bbcd4ef 
Signed-off-by: John Molakvoæ <[email protected]>
@skjnldsv skjnldsv mentioned this pull request Nov 9, 2022
Open
@skjnldsv skjnldsv requested a review from juliusknorr November 9, 2022 10:21
@skjnldsv skjnldsv requested review from Raudius and julien-nc November 9, 2022 10:21
skjnldsv
skjnldsv commented Nov 9, 2022
View reviewed changes
src/views/Viewer.vue

// 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 •
edited
Loading

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

@Raudius Raudius Nov 9, 2022

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

@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.

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

viewer/src/views/Viewer.vue

Line 613 in bbcd4ef

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

viewer/src/views/Viewer.vue

Line 637 in bbcd4ef

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

viewer/src/views/Viewer.vue

Line 647 in bbcd4ef

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

@jkhsjdhjs
Copy link

jkhsjdhjs commented Nov 9, 2022

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 mentioned this pull request Mar 7, 2023
Merged
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@skjnldsv skjnldsv mentioned this pull request May 3, 2023
Merged
@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

ariselseng commented Aug 29, 2023

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.

@ariselseng ariselseng mentioned this pull request Sep 17, 2023
Closed
@joshtrichards
Copy link
Member

joshtrichards commented Oct 7, 2023

@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?

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Mar 9, 2024
This was referenced Mar 12, 2024
Merged
Merged
Merged
@Altahrim Altahrim mentioned this pull request Mar 20, 2024
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30.0.11 milestone May 15, 2025
@skjnldsv skjnldsv closed this Jun 1, 2025
@susnux susnux deleted the fix/aliases branch October 10, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@Raudius Raudius Awaiting requested review from Raudius

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees

@skjnldsv skjnldsv

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RAW photos are not displayed with infinite spinner

7 participants

@skjnldsv @jkhsjdhjs @Raudius @ariselseng @joshtrichards @blizzz @Altahrim