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 repeated exceptions in Bookmarked Playlists #3345

Merged
merged 1 commit into from
Apr 19, 2020

Conversation

mitosagi
Copy link
Contributor

@mitosagi mitosagi commented Apr 3, 2020

What is it?

  • Bug fix
  • Feature

Long description of the changes in your PR

This is a fix for #3261 (App loops in null pointer with bookmarks exception).
A fix for a bug that prevented the Bookmarked Playlists screen from being displayed because the playlist name had changed to null for some reason.
When Bookmarked Playlists is displayed, sorting is done against the playlist names. This PR prevents crashes by allowing you to sort playlist names that contain a null.

The fundamental solution is to prevent null in playlist names, but I think this fix is necessary to open a database that is already in an unopenable state.

Fixes the following issue(s)

Testing apk

apk: errorwithbookmarks.zip
database: NewPipeData-Test.zip

Steps to reproduce the behavior:

  1. Import this database.
  2. Open the Bookmarked Playlists screen.

Agreement

@mitosagi mitosagi changed the title Fix repeated exceptions in Bookmarked Playlists. Fix repeated exceptions in Bookmarked Playlists Apr 3, 2020
@wb9688
Copy link
Contributor

wb9688 commented Apr 3, 2020

You still haven't fixed all coding style issues. Also, we prefer rebasing instead of having a lot of merge commits.

@TobiGr
Copy link
Contributor

TobiGr commented Apr 4, 2020

Thank you for the fix. Please squash your commits

@mitosagi mitosagi force-pushed the error-with-bookmarks branch from 067a054 to 4491b66 Compare April 5, 2020 08:28
@TobiGr TobiGr added this to the 0.19.3 milestone Apr 6, 2020
@wb9688
Copy link
Contributor

wb9688 commented Apr 8, 2020

The code itself is OK now.

@mauriciocolli: Shouldn't it be impossible for playlist names to be null? If so, could you write a migration instead?

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Thank you 👍
I'll merge this for now to fix the crash in the next version. We can revert that cahnge later once we have a migration.

@TobiGr TobiGr merged commit cd53518 into TeamNewPipe:dev Apr 19, 2020
@mitosagi mitosagi deleted the error-with-bookmarks branch April 20, 2020 13:17
This was referenced Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App loops in null pointer with bookmarks exception.
3 participants