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

Create an app-wide Disposable to be used for app-wide actions #7942

Closed
5 tasks done
Stypox opened this issue Feb 24, 2022 · 2 comments
Closed
5 tasks done

Create an app-wide Disposable to be used for app-wide actions #7942

Stypox opened this issue Feb 24, 2022 · 2 comments
Labels
codequality Improvements to the codebase to improve the code quality wontfix This issue will not be fixed

Comments

@Stypox
Copy link
Member

Stypox commented Feb 24, 2022

Checklist

  • This issue contains only one bug.
  • I am able to reproduce the bug with the latest version.
  • I have read and understood the contribution guidelines.
  • I made sure that there are no existing issues - open or closed - which I could contribute my information to.
  • I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.

Affected version

After (but also before) #7570

The problem

In the NewPipe codebase there are many methods that use RxJava to perform resource/time/io-heavy actions in the background. When these actions finally result in a change in the activity/fragment/dialog that originally triggered them, then an RxJava Disposable should be passed to those functions, so that if the activity/fragment/dialog is dismissed before the action can complete, the action can be dispose()d. This is what is correctly being done in the text linkifier class: they take a text view and set its text once it has been linkified, but if in the meantime the activity/fragment/dialog associated to the text view was destroyed, the linkification of the text will stop (otherwise it would result in calling setText() on a non-existing text view).

Sometimes, though, an action is not really associated with the activity/fragment/dialog that triggered it. If the user want to start playing something in a popup, he may tap on the popup button in the long-press menu of the feed fragment, and then maybe he instantly (i.e. before the popup player had a chance to load) closes the feed fragment to switch to another app. If the disposable for playing in popup was added to the Disposables associated with the feed fragment, the loading of the popup player would stop. So currently what is being done instead is basically just not saving disposables anywhere so they are never disposed, for example checkout this. Though this behavior mostly works fine, I don't think it is a good practice and may lead to problems, so I think we should create an app-wide Disposable to which all disposables related to app-wide actions can be added.

@Stypox Stypox added bug Issue is related to a bug codequality Improvements to the codebase to improve the code quality labels Feb 24, 2022
@AudricV
Copy link
Member

AudricV commented Feb 11, 2024

@Stypox As we want to rewrite the app and migrate it to Kotlin and use coroutines, is this issue still relevant?

@AudricV AudricV added the waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed. label Feb 11, 2024
@Stypox
Copy link
Member Author

Stypox commented Feb 12, 2024

No, and it's also a bad idea. We should use dependency injection.

@github-actions github-actions bot removed the waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed. label Feb 12, 2024
@Stypox Stypox closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2024
@AudricV AudricV added wontfix This issue will not be fixed and removed bug Issue is related to a bug labels Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codequality Improvements to the codebase to improve the code quality wontfix This issue will not be fixed
Projects
None yet
Development

No branches or pull requests

2 participants