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.

Sign up for GitHub

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

Jump to bottom

[Issue]: Can't manually add or delete subtitles #11318

Open
1 task done
Simsala91 opened this issue Apr 8, 2024 · 7 comments · May be fixed by #11507
Open
1 task done

[Issue]: Can't manually add or delete subtitles #11318

Simsala91 opened this issue Apr 8, 2024 · 7 comments · May be fixed by #11507
Labels
bug Something isn't working
Projects
Issue Triage for Main Repo

Comments

@Simsala91
Copy link

Simsala91 commented Apr 8, 2024 •
edited

Please describe your bug

When editing external subtitles of a movie, it is neither possible to manually add a subtitle nor is it possible to manually delete subtitles as a non-admin user with the permission "Allow user to edit subtitles".

Adding subtitles with the OpenSubtitlesPlugin works fine.

Reproduction Steps

(Scenario A) Have a movie with external subtitles. Try to remove them as a non-admin user with permission "Allow user to edit subtitles". Subtitle does not get deleted. (This works as admin.)

(Scenario B) Try to manually upload a subtitle file either as admin or as non-admin with permission "Allow user to edit subtitles". The subtitle does not get created. (Also after refresh).

Jellyfin Version

Unstable (master branch)

if other:

No response

Environment

- OS: Unraid
- Linux Kernel: unknown
- Virtualization: Docker
- Clients: Browser
- Browser: Firefox
- FFmpeg Version: -
- Playback Method: -
- Hardware Acceleration: -
- GPU Model: -
- Plugins: OpenSubtitles
- Reverse Proxy: none
- Base URL: none
- Networking:  Bridge
- Storage: local
- Build: 2024040805

Jellyfin logs

[16:09:45] [INF] [31] Emby.Server.Implementations.HttpServer.WebSocketManager: WS 192.168.178.25 closed
[16:09:45] [ERR] [36] Jellyfin.Api.Middleware.ExceptionMiddleware: Error processing request: Unexpected end of request content. URL POST /Videos/8194315094fc0ce43369e0d71eac34b0/Subtitles.
[16:09:46] [INF] [50] Emby.Server.Implementations.HttpServer.WebSocketManager: WS 192.168.178.25 request

FFmpeg logs

No response

Please attach any browser or client logs here

No response

Please attach any screenshots here

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Simsala91 Simsala91 added the bug Something isn't working label Apr 8, 2024
@jellyfin-bot
Copy link
Contributor

Hi, it seems like your issue report has the following item(s) that need to be addressed:

  • The format of the environment section has been altered from the template.

This is an automated message, currently under testing. Please file an issue here if you encounter any problems.

@Simsala91 Simsala91 mentioned this issue Apr 8, 2024
Merged
@Ch1nkara
Copy link
Contributor

Ch1nkara commented Apr 9, 2024

Hi, thanks for the feedback. Indeed, I overlooked that part in jellyfin/jellyfin-web#4883. I will happily dig into it and offer a fix. Might not have time to work on it before the end of this month so if someone else wants to step up, don't hesitate

@nielsvanvelzen
Copy link
Member

The backend change is not as simple as adding the right policy to the delete endpoint though. We should only allow the deletion of remote subtitles with the EnableSubtitleManagement policy, subtitles already in the media directory should be left untouched.

@jellyfin-bot jellyfin-bot added this to Needs triage in Issue Triage for Main Repo Apr 9, 2024
@Ch1nkara
Copy link
Contributor

Ch1nkara commented Apr 9, 2024

To be honest, I don't really agree with that. I think the EnableSubtitleManagement is there to give administrator rigths over the subtitles without giving full administrator rights. What you"re suggesting seems more confusing. And more complex to implement.

@Simsala91
Copy link
Author

Personally, I also think this should also allow the deletion of external subtitle files. If I give another user that permission, I trust that user to pick the correct subtitles. Sometimes automatic subtitle downloader will download a subtitle that is out of sync, and I would like users (that I trust) to be able to fix that without having to contact me.

Ch1nkara added a commit to Ch1nkara/jellyfin that referenced this issue May 7, 2024
@Ch1nkara
fix jellyfin#11318 by editing the delete endpoint
9731be0
@Ch1nkara Ch1nkara linked a pull request May 7, 2024 that will close this issue
Open
@Ch1nkara
Copy link
Contributor

Ch1nkara commented May 7, 2024

I made a PR to propose a fix. it allows a non-admin user and admin users to delete any external subtitle. As discussed, it is the approach I prefer. I'll let the jellyfin core team decide if they accept it or not.

This would solve (Scenario A) of this issue.

@Simsala91, I was unable to reproduce (Senario B) issue. I had no trouble manually importing subtitles as both admin and non-admin user, via click and drop and browsing

@Simsala91
Copy link
Author

@Ch1nkara Thanks for the work! Also can't reproduce Scenario B on current branch. Might have been fixed by #11310 or #11321
or #11373

Ch1nkara added a commit to Ch1nkara/jellyfin that referenced this issue May 17, 2024
@Ch1nkara
fix jellyfin#11318 by editing the delete endpoint
e42e8fe
@Ch1nkara Ch1nkara mentioned this issue May 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Issue Triage for Main Repo
Needs triage
Jellyfin - Triage Board
Status: Team Review
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix deleting subtitles with new SubtitleManagement permission Ch1nkara/jellyfin
4 participants
@nielsvanvelzen @Simsala91 @jellyfin-bot @Ch1nkara