-
-
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
Update AndroidX Fragment to 1.3.4 #6394
Conversation
Also, I'm not that great at naming things, so feel free to tell me if you have better names in mind for the objects and functions and I'll rename them. |
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.
I've read the changelog, but I do not know the architecture of fragments well enough to understand the changelog and your changes thoroughly.
(btw, do you happen to have some up-to-date and complete guide to fragments? I couldn't find good-quality material about it)
app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt
Show resolved
Hide resolved
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.
LGTM, thanks :-D
stracktrace: search``` 06-18 15:07:55.443 5829-5829/org.schabi.newpipe.debug E/ACRA: ACRA caught a NullPointerException for org.schabi.newpipe.debug java.lang.NullPointerException at org.schabi.newpipe.fragments.list.search.SearchFragment.onCreateOptionsMenu(SearchFragment.java:430) at androidx.fragment.app.Fragment.performCreateOptionsMenu(Fragment.java:3100) at androidx.fragment.app.FragmentManager.dispatchCreateOptionsMenu(FragmentManager.java:3181) at androidx.fragment.app.FragmentController.dispatchCreateOptionsMenu(FragmentController.java:391) at androidx.fragment.app.FragmentActivity.onCreatePanelMenu(FragmentActivity.java:288) at androidx.appcompat.view.WindowCallbackWrapper.onCreatePanelMenu(WindowCallbackWrapper.java:94) at androidx.appcompat.app.AppCompatDelegateImpl$AppCompatWindowCallback.onCreatePanelMenu(AppCompatDelegateImpl.java:3070) at androidx.appcompat.view.WindowCallbackWrapper.onCreatePanelMenu(WindowCallbackWrapper.java:94) at androidx.appcompat.app.ToolbarActionBar.populateOptionsMenu(ToolbarActionBar.java:456) at androidx.appcompat.app.ToolbarActionBar$1.run(ToolbarActionBar.java:57) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:761) at android.view.Choreographer.doCallbacks(Choreographer.java:574) at android.view.Choreographer.doFrame(Choreographer.java:543) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:747) at android.os.Handler.handleCallback(Handler.java:733) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:136) at android.app.ActivityThread.main(ActivityThread.java:5017) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:515) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595) at dalvik.system.NativeStart.main(Native Method) ``` |
@TobiGr Are you sure that's related to this PR specifically? I didn't touch anything related to the search function... EDIT: Actually the NPE might have to do with new nullabilities that may have been introduced between 1.2.4 and 1.3.4. |
Seems like it: #6522. It specifically refers to your PR. |
@opusforlife2 He may just be referring to 0.21.5. As of right now, Stypox's merge of my PR is the latest commit. Guess we'll have to wait for a response 🤔 |
I don't really understand this stuff and I don't see any mentions of changes to Fragment lifecycle here (https://developer.android.com/guide/fragments/lifecycle) but I noticed in SearchFragment.java service is only set in onResume. Copying the exact code from onResume ( try {
service = NewPipe.getService(serviceId);
} catch (final Exception e) {
ErrorActivity.reportUiErrorInSnackbar(this,
"Getting service for id " + serviceId, e);
} ) to onCreate means the crash (#6522) in SearchFragment no longer occurs. idk whether we still need it in onResume or whether this could cause problems in other fragments so I will leave that to you guys. |
…ragment It seems due to TeamNewPipe#6394 updating the FragmentX library there was a change to the order of lifecycle calls, as such onResume() was no longer before onCreateOptionsMenu() creating a null pointer exception when using service in onCreateOptionsMenu() as it is only set in onResume(). By moving the initialization of service to onStart() which still happens before onCreateOptionsMenu() this crash can be avoided. This commit also adds a check for a null service to prevent future crashes for similar issues.
What is it?
Description of the changes in your PR
Fixes the following issue(s)
APK testing
On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.
Due diligence
Notes
There are other deprecations that come with this upgrade that will need fixing at some point:
I didn't do other deprecations like onRequestPermissionsResult and the other onActivityResult ones because I'm not too confident and I want to keep this one smaller. I'll do them in other PRs.