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.

Sign up for GitHub

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

Jump to bottom

bug fix 5240 #5292

Merged
merged 3 commits into from
Sep 25, 2023
Merged

bug fix 5240 #5292

merged 3 commits into from
Sep 25, 2023

Conversation

alamzeeshan
Copy link
Contributor

Description (required)

Fixes #5240

What changes did you make and why?
Different activities and fragments were invoking MediaDetailPagerFragment by calling its parameterized constructor but in Android we should be passing arguments to a Fragment via Bundle, so made these changes

Tests performed (required)
app/src/test/kotlin/fr/free/nrw/commons/media/MediaDetailPagerFragmentUnitTests.kt

Tested {build variant, e.g. ProdDebug} on {name of device or emulator} with API level {API level}.

Screenshots (for UI changes only)

Need help? See https://support.google.com/android/answer/9075928

Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

@alamzeeshan
bug fix 5240
293296d 
Signed-off-by: Zeeshan Alam <zeeshan.alam@fmr.com>
@nicolas-raoul
Copy link
Member

I have been using this branch today and will continue tomorrow.
I did not encounter the bug before either, but at least this branch does not seems to trigger any additional issue. 🙂

@alamzeeshan
Incorporated review comments. Remove method MediaDetailPagerFragment.…
1dcc970 
…newInstance(false, false, 0) which was called only in unit test
@nicolas-raoul
Copy link
Member

Probably just a coincidence but I just observed this media details bug for the first time, with this branch. Could it be related to this pull request, by any chance?

@nicolas-raoul nicolas-raoul mentioned this pull request Sep 17, 2023
Closed
@alamzeeshan
Copy link
Contributor Author

Probably just a coincidence but I just observed this media details bug for the first time, with this branch. Could it be related to this pull request, by any chance?

I don't think so. I tried to replicate the issue but I was unable to.

@nicolas-raoul
Copy link
Member

nicolas-raoul commented Sep 18, 2023 •
edited
Loading

Yesterday I got this crash using this branch (upoading 20 pictures, then leaving the phone and coming back to it anhour or so later), I wonder whether it is related?

APP_VERSION_CODE=1034
APP_VERSION_NAME=4.1.0-debug-fidelity-zeeshan_branch
ANDROID_VERSION=14
PHONE_MODEL=Pixel 7 Pro
STACK_TRACE=java.lang.RuntimeException: Unable to start activity ComponentInfo{fr.free.nrw.commons/fr.free.nrw.commons.contributions.MainActivity}: androidx.fragment.app.Fragment$InstantiationException: Unable to instantiate fragment fr.free.nrw.commons.media.MediaDetailPagerFragment: could not find Fragment constructor
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3782)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3922)
at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103)
at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:139)
at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:96)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2443)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loopOnce(Looper.java:205)
at android.os.Looper.loop(Looper.java:294)
at android.app.ActivityThread.main(ActivityThread.java:8177)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)
Caused by: androidx.fragment.app.Fragment$InstantiationException: Unable to instantiate fragment fr.free.nrw.commons.media.MediaDetailPagerFragment: could not find Fragment constructor
at androidx.fragment.app.Fragment.instantiate(Fragment.java:628)
at androidx.fragment.app.FragmentContainer.instantiate(FragmentContainer.java:57)
at androidx.fragment.app.FragmentManager$3.instantiate(FragmentManager.java:483)
at androidx.fragment.app.FragmentStateManager.<init>(FragmentStateManager.java:85)
at androidx.fragment.app.FragmentManager.restoreSaveState(FragmentManager.java:2728)
at androidx.fragment.app.Fragment.restoreChildFragmentState(Fragment.java:1890)
at androidx.fragment.app.Fragment.onCreate(Fragment.java:1867)
at fr.free.nrw.commons.contributions.ContributionsFragment.onCreate(ContributionsFragment.java:132)
at androidx.fragment.app.Fragment.performCreate(Fragment.java:2949)
at androidx.fragment.app.FragmentStateManager.create(FragmentStateManager.java:475)
at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:278)
at androidx.fragment.app.FragmentStore.moveToExpectedState(FragmentStore.java:112)
at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1647)
at androidx.fragment.app.FragmentManager.dispatchStateChange(FragmentManager.java:3128)
at androidx.fragment.app.FragmentManager.dispatchCreate(FragmentManager.java:3061)
at androidx.fragment.app.FragmentController.dispatchCreate(FragmentController.java:240)
at androidx.fragment.app.FragmentActivity.onCreate(FragmentActivity.java:276)
at androidx.appcompat.app.AppCompatActivity.onCreate(AppCompatActivity.java:106)
at fr.free.nrw.commons.di.CommonsDaggerAppCompatActivity.onCreate(CommonsDaggerAppCompatActivity.java:24)
at fr.free.nrw.commons.theme.BaseActivity.onCreate(BaseActivity.java:28)
at fr.free.nrw.commons.contributions.MainActivity.onCreate(MainActivity.java:122)
at android.app.Activity.performCreate(Activity.java:8595)
at android.app.Activity.performCreate(Activity.java:8573)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1456)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3764)
... 12 more
Caused by: java.lang.NoSuchMethodException: fr.free.nrw.commons.media.MediaDetailPagerFragment.<init> []
at java.lang.Class.getConstructor0(Class.java:3325)
at java.lang.Class.getConstructor(Class.java:2485)
at androidx.fragment.app.Fragment.instantiate(Fragment.java:613)
... 36 more

IS_SILENT=false
REPORT_ID=09961c6a-a91b-4625-81a7-82f826c9dd7d
USER_CRASH_DATE=2023-09-17T19:02:36.503+09:00

I have already experienced this error in June, July, August, and 4 times this month on other branches, so at least it is probably not a consequence of this pull request.

@alamzeeshan
Copy link
Contributor Author

Yesterday I got this crash using this branch (upoading 20 pictures, then leaving the phone and coming back to it anhour or so later), I wonder whether it is related?

APP_VERSION_CODE=1034
APP_VERSION_NAME=4.1.0-debug-fidelity-zeeshan_branch
ANDROID_VERSION=14
PHONE_MODEL=Pixel 7 Pro
STACK_TRACE=java.lang.RuntimeException: Unable to start activity ComponentInfo{fr.free.nrw.commons/fr.free.nrw.commons.contributions.MainActivity}: androidx.fragment.app.Fragment$InstantiationException: Unable to instantiate fragment fr.free.nrw.commons.media.MediaDetailPagerFragment: could not find Fragment constructor
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3782)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3922)
at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103)
at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:139)
at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:96)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2443)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loopOnce(Looper.java:205)
at android.os.Looper.loop(Looper.java:294)
at android.app.ActivityThread.main(ActivityThread.java:8177)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)
Caused by: androidx.fragment.app.Fragment$InstantiationException: Unable to instantiate fragment fr.free.nrw.commons.media.MediaDetailPagerFragment: could not find Fragment constructor
at androidx.fragment.app.Fragment.instantiate(Fragment.java:628)
at androidx.fragment.app.FragmentContainer.instantiate(FragmentContainer.java:57)
at androidx.fragment.app.FragmentManager$3.instantiate(FragmentManager.java:483)
at androidx.fragment.app.FragmentStateManager.<init>(FragmentStateManager.java:85)
at androidx.fragment.app.FragmentManager.restoreSaveState(FragmentManager.java:2728)
at androidx.fragment.app.Fragment.restoreChildFragmentState(Fragment.java:1890)
at androidx.fragment.app.Fragment.onCreate(Fragment.java:1867)
at fr.free.nrw.commons.contributions.ContributionsFragment.onCreate(ContributionsFragment.java:132)
at androidx.fragment.app.Fragment.performCreate(Fragment.java:2949)
at androidx.fragment.app.FragmentStateManager.create(FragmentStateManager.java:475)
at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:278)
at androidx.fragment.app.FragmentStore.moveToExpectedState(FragmentStore.java:112)
at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1647)
at androidx.fragment.app.FragmentManager.dispatchStateChange(FragmentManager.java:3128)
at androidx.fragment.app.FragmentManager.dispatchCreate(FragmentManager.java:3061)
at androidx.fragment.app.FragmentController.dispatchCreate(FragmentController.java:240)
at androidx.fragment.app.FragmentActivity.onCreate(FragmentActivity.java:276)
at androidx.appcompat.app.AppCompatActivity.onCreate(AppCompatActivity.java:106)
at fr.free.nrw.commons.di.CommonsDaggerAppCompatActivity.onCreate(CommonsDaggerAppCompatActivity.java:24)
at fr.free.nrw.commons.theme.BaseActivity.onCreate(BaseActivity.java:28)
at fr.free.nrw.commons.contributions.MainActivity.onCreate(MainActivity.java:122)
at android.app.Activity.performCreate(Activity.java:8595)
at android.app.Activity.performCreate(Activity.java:8573)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1456)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3764)
... 12 more
Caused by: java.lang.NoSuchMethodException: fr.free.nrw.commons.media.MediaDetailPagerFragment.<init> []
at java.lang.Class.getConstructor0(Class.java:3325)
at java.lang.Class.getConstructor(Class.java:2485)
at androidx.fragment.app.Fragment.instantiate(Fragment.java:613)
... 36 more

IS_SILENT=false
REPORT_ID=09961c6a-a91b-4625-81a7-82f826c9dd7d
USER_CRASH_DATE=2023-09-17T19:02:36.503+09:00

I have already experienced this error in June, July, August, and 4 times this month on other branches, so at least it is probably not a consequence of this pull request.

This is happing because of the private constructor. I have made the constructor public. This should fix the issue.

@nicolas-raoul
Copy link
Member

Thanks, I just installed this version and start testing now. :-)

alamzeeshan

This comment was marked as outdated.

Sign in to view

nicolas-raoul
nicolas-raoul approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

I haven't been able to reproduce the crash with this version.
I only tried for one day (about 10 multi-uploads) though and even before occurrences were rare, so I can't tell for sure that it is fixed, but I now need to test other pull requests.
How about merging and coming back to this if there are new occurrences?

@alamzeeshan
Copy link
Contributor Author

I haven't been able to reproduce the crash with this version. I only tried for one day (about 10 multi-uploads) though and even before occurrences were rare, so I can't tell for sure that it is fixed, but I now need to test other pull requests. How about merging and coming back to this if there are new occurrences?

Sure. We will revisit this issue if this happens again.

@alamzeeshan alamzeeshan reopened this Sep 19, 2023
@alamzeeshan
Copy link
Contributor Author

@nicolas-raoul Are we good with these changes? Just saw that the changes are not merged with the main branch.

@sivaraam sivaraam merged commit 19733b3 into commons-app:main Sep 25, 2023
2 checks passed
@sivaraam
Copy link
Member

sivaraam commented Sep 25, 2023 •
edited
Loading

Thank you for your contribution! @alamzeeshan

@nicolas-raoul
Copy link
Member

@alamzeeshan: Oops sorry and thanks for the ping! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RitikaPahwa4444 RitikaPahwa4444 RitikaPahwa4444 left review comments

@sivaraam sivaraam sivaraam left review comments

@nicolas-raoul nicolas-raoul nicolas-raoul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Crash in MediaDetailPagerFragment due to lack of no-arg constructor
4 participants
@alamzeeshan @nicolas-raoul @sivaraam @RitikaPahwa4444