NewPipe
NewPipe copied to clipboard
Set compileSdk and targetSdk to 33 (Android 13)
What is it?
- [x] Bugfix (user facing)
- [ ] Feature (user facing)
- [x] Codebase improvement (dev facing)
- [ ] Meta improvement to the project (dev facing)
Description of the changes in your PR
android:exported
in now required in the manifest on all activities/services/receivers/providers. It was set to true
for those that need to interact with outside apps or the OS, while others have exported=false
. Changing the target sdk also required updating LeakCanary to the latest version as the older version being used was not using android:exported
in AndroidManifest.xml
.
I don't know whether problems could arise just because of this number increase, maybe @TacoTheDank can shed some light on it?
Fixes the following issue(s)
- https://github.com/TeamNewPipe/NewPipe/issues/9292#issuecomment-1304571423
Relies on the following changes
- #9333
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
- [x] I read the contribution guidelines.
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells
No Coverage information
0.0% Duplication
Opening any video crashes the app.
Exception
- User Action: ui error
- Request: ACRA report
- Content Country: US
- Content Language: en-US
- App Language: en_US
- Service: none
- Version: 0.24.0
- OS: Linux Android 13 - 33
Crash log
java.lang.IllegalArgumentException: org.schabi.newpipe.debug.targetapi33: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
at android.app.PendingIntent.checkFlags(PendingIntent.java:401)
at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:671)
at android.app.PendingIntent.getBroadcast(PendingIntent.java:658)
at org.schabi.newpipe.player.notification.NotificationUtil.createNotification(NotificationUtil.java:136)
at org.schabi.newpipe.player.notification.NotificationUtil.createNotificationAndStartForeground(NotificationUtil.java:185)
at org.schabi.newpipe.player.notification.NotificationPlayerUi.initPlayer(NotificationPlayerUi.java:32)
at org.schabi.newpipe.player.Player$$ExternalSyntheticLambda0.accept(Unknown Source:2)
at j$.util.Iterator$-CC.$default$forEachRemaining(Iterator.java:116)
at j$.util.Iterator$-EL.forEachRemaining(Unknown Source:10)
at j$.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
at j$.util.stream.ReferencePipeline$Head.forEachOrdered(ReferencePipeline.java:590)
at org.schabi.newpipe.player.ui.PlayerUiList.call(PlayerUiList.java:88)
at org.schabi.newpipe.player.Player.initPlayer(Player.java:522)
at org.schabi.newpipe.player.Player.initPlayback(Player.java:487)
at org.schabi.newpipe.player.Player.lambda$handleIntent$2$org-schabi-newpipe-player-Player(Player.java:425)
at org.schabi.newpipe.player.Player$$ExternalSyntheticLambda27.run(Unknown Source:16)
at io.reactivex.rxjava3.internal.operators.maybe.MaybeCallbackObserver.onComplete(MaybeCallbackObserver.java:93)
at io.reactivex.rxjava3.internal.operators.maybe.MaybeObserveOn$ObserveOnMaybeObserver.run(MaybeObserveOn.java:105)
at io.reactivex.rxjava3.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:123)
at android.os.Handler.handleCallback(Handler.java:942)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loopOnce(Looper.java:201)
at android.os.Looper.loop(Looper.java:288)
at android.app.ActivityThread.main(ActivityThread.java:7898)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
If this pull request was merged, a new version would be released.
@Nizami20052022 Please be aware that you are creating a lot of comments and new issues. Many of them do not add any value to the project or its discussions and are just consuming resources for reviewing and replying. Our resources are limited. Please make sure to stick to our contribution guidelines. If you fail to do so, we might need to restrict your ability to interact with our repositories to not bind any more resources.
I haven't opened many issues.😪
I report problems.I will not open any more issues. Sorry!😪
@Nizami20052022 It's more about the amount of effort it takes to deal with it. If a lot of your opened issues are being closed, and your comments being marked off-topic, that's an indication for you to read more and comment less. It will improve the quality of your interactions in FOSS repos in general, too.
If you allow me, I can create my own pull request.
PRs are always welcome, as we've accepted yours previously. There's no permission needed. Just make sure the code is of good quality and follow our contribution guidelines.
I can contribute other than translation.I finished the translation.Don't worry about code quality.
I don't know whether problems could arise just because of this number increase, maybe TacoTheDank can shed some light on it?
I recommend reading Android version differences (API 30 - changes when targeting this version, API 31 - changes when targeting this version, API 32 and API 33 - changes when targeting this version) and changelog of Jetpack libraries updates, to give us an idea about what changes are required.
I second @AudricV's suggestion.
Opening any video crashes the app.
The PendingIntent
mutability flags are required when targeting Android 12 and higher, and most of the PendingIntent
s created in the app don't have the flags set.
You may also want to update com.android.tools:desugar_jdk_libs
to 1.1.8 to target API level 33. (https://github.com/google/desugar_jdk_libs/blob/master/CHANGELOG.md)
You may also want to update
com.android.tools:desugar_jdk_libs
to 1.1.8 to target API level 33. (https://github.com/google/desugar_jdk_libs/blob/master/CHANGELOG.md)
That's done here: #9285
I pushed two commits making sure that NewPipe now requests the notification permission, as explained here.
On Android 13+ we need to specify player notification actions with the PlaybackState
class, see here. This can be done in a separate PR I think, since currently the custom actions are not shown on Android 13 regardless of whether the app targets 29 or 33.
The code smells are unrelated
The code smells are unrelated
No, they are: see https://developer.android.com/reference/android/app/Activity.html?hl=en#onBackPressed() and https://developer.android.com/reference/android/content/Intent?hl=en#getSerializableExtra(java.lang.String)
These methods are deprecated since API 33 and used in MainActivity
.
I don't think I am going to fix those deprecations here.
For example, getSerializableExtra(String)
suggests getSerializableExtra(String, class)
as a replacement, but the latter requires API 33 and I couldn't find a similar method in IntentCompat
, so it's not solvable as of now. If you or @Isira-Seneviratne find a solution feel free to push commits to this PR and then merge it; otherwise please merge it as it is and we will fix deprecations once IntentCompat
catches up.
As for the onBackPressed
deprecations, I am not sure how to proceed. There seems to be too many changes to do in this PR. I think it's better to do them in another PR.
I took a deeper look into the onBackPressed
deprecation. When we just need to always consume the onBack
it is simple to switch to just register a callback that is always called. But when you have a conditional onBack
(e.g. close the drawer or the search bar if it is open, otherwise let super
handle the onBack
), this requires enabling or disabling the callback based on whether the drawer or the search bar are open. This means having to continuously sync the "openness" state of drawer/search with the "enabledness" state of the onBack
callback. Not only does this require much more code than before, it also greatly enlarges the possibility for programming errors and non-synced state. Using enableable/disableable onBack
callbacks in a state-based UI system like Jetpack Compose would work perfectly well, but for us it would be a nightmare. So I will merge this PR as it is.