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 crash when the user clicks download then quits the history fragment #9143

Merged

Conversation

plasticanu
Copy link
Contributor

@plasticanu plasticanu commented Oct 17, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Add a condition to detect if the user quits the current fragment. If the user clicked "Download" in the history fragment and then quit the history fragment, a DownloadDialog will not show and not crash the program.

Before/After Screenshots/Screen Record

  • Before:
193872207-777dbbe8-256d-4e37-b0eb-8a85939ded64.mp4
  • After:
Screen.Recording.2022-10-18.at.12.18.05.am.mov

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@TobiGr TobiGr added bug Issue is related to a bug downloader Issue is related to the downloader labels Oct 17, 2022
@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TobiGr
Copy link
Member

TobiGr commented Oct 19, 2022

Thank you. While you are at it, please add a @Nonnull annotation to the context parameter in the DownloadDialog constructor.

@plasticanu
Copy link
Contributor Author

Thank you. While you are at it, please add a @Nonnull annotation to the context parameter in the DownloadDialog constructor.

Done.

@plasticanu
Copy link
Contributor Author

plasticanu commented Oct 25, 2022

Hi, I merged the latest dev from upstream to my branch, and then I can't pass the CI. I have to revert the merge, but then my pull request will change the build files to the previous version than the latest dev.
Is there anything I can do? I am not sure why the CI failed, it says can't find newPipe Extractor.

…adDialogCrash"

This reverts commit 968d7a7, reversing
changes made to 52963ba.

Reverted merge

jlhzxc
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Please revert the changes in both build.gradle and both AndroidManifest.xml. The other code looks good to me, thank you for taking care of this :-D. Don't worry about the CI failing, it's not your fault, but it seems to be a general problem of Jitpack.

@plasticanu
Copy link
Contributor Author

Please revert the changes in both build.gradle and both AndroidManifest.xml. The other code looks good to me, thank you for taking care of this :-D. Don't worry about the CI failing, it's not your fault, but it seems to be a general problem of Jitpack.

Thanks. Done

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thanks ;-)
Note that the null check could be extended to all other stream dialog entries: not only the download dialog could cause crashes. If you want to take care of that, please open a new PR.
I tested the changes myself as the CI isn't working at the moment.

@Stypox Stypox merged commit 4081508 into TeamNewPipe:dev Oct 26, 2022
This was referenced Nov 4, 2022
@plasticanu plasticanu deleted the fix/HistoryFragmentDownloadDialogCrash branch January 4, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug downloader Issue is related to the downloader
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download error.
3 participants