-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 long-pressing stream while player is starting #7704
Conversation
Fix NullPointerException in PlayerHolder.getQueueSize() and add `@Nullable` here and there so that the linter reports risks of NPEs
Seems to work. Its now extremely fast, so I dont know weather I reproduced the issue exactly but no crashes yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
just for records, this does fix not #7608 , i have given my input over that issue itself |
@yashpalgoyal1304 I fixed your problem in the last commit, please check again the APK ;-) |
SonarCloud Quality Gate failed. |
Ignore the failure by sonarcloud, I just had to edit many duplicated lines, which will be solved by #7570 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM 😄
However I didn't test it so far...
I tested it, and imo, showing the Enqueue option but graying it out (with properly chosen colors) will be more indicative and more fool proof. what do others think?? |
@yashpalgoyal1304 In my opinion not showing the option is enough:
|
fair 👍 (: |
So this PR does not fix the problem, just masks it by hiding the "Enqueue" menu option while loading? And if I want to quickly enqueue several videos, I have to wait while it loads? |
It fixes the crash by not allowing to enqueue anything while the player is not ready for it.
I assume so. But it shouldn't happen so often in the first place. See also → We/You can open an additional feature request after it was fixed 😄(from #7704 (comment)) |
What is it?
Description of the changes in your PR
This PR fixes a NullPointerException in
PlayerHolder.getQueueSize()
that could happen when the player's queue isnull
, i.e. when the player is starting. It also adds@Nullable
here and there so that Android Studio's linter reports risks of NPEs.I think this PR should be merged before the next release as it's just adding an
if (... != null)
but solves a big issue.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