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: IndexOutOfBoundsException crash when removing last two images of multiupload #6124

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

rohit9625
Copy link
Contributor

Description (required)

Fixes #6122

What changes did you make and why?
When removing images from Thumbnails Row, the uploadableImages property inside ThubnailsAdapter was not updating. It results in the image being removed at an invalid index and sometimes IndexOutOfBoundsException.
So, I added the code to notify the adapter that the image was removed.

Tests performed (required)

Tested ProdDebug on Samsung A14 with API level 34.

Screenshots (for UI changes only)
Screencast after fix

After_Fix.mp4

@nicolas-raoul
Copy link
Member

Looks great, thanks!
Sorry I just merged a pull request with UploadActivity.java moved to Kotlin, would you mind porting the change?

@rohit9625
Copy link
Contributor Author

Done 👍

rohit9625 and others added 8 commits January 14, 2025 10:51
Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
* Rename .java to .kt

* Migrate Profile Package to Kotlin commons-app#6118 commons-app#5928

* Migrate Profile Package to Kotlin commons-app#6118 commons-app#5928

* Migrate Profile Package to Kotlin commons-app#6118 commons-app#5928
Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
…, then suggest categories from its P18 (commons-app#6130)

* Fix NPE with UploadMediaDetails.captionText

* Store P18 instead of processed image url in DepictedItem

* Add routes for fetching category info from titles

* Consider depict's P18 when suggesting categories

* Add tests

* Corrected DepictedItem constructor arguments

* Add test for DepictedItem::primaryImage
…-app#6139)

* fix: correctly handle permission callbacks on Main thread

The PermissionUtils was incorrectly executing permission callbacks on a background thread, leading to a Handler error. Now, it is using Main dispatcher.

* fix crash when opening camera while having partial storage access
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 am getting this crash when trying to remove the middle picture, any idea what might be the issue? 🙂

APP_VERSION_NAME=5.1.2-debug-rohit9625-fix-multiupload
ANDROID_VERSION=15
PHONE_MODEL=Pixel 9 Pro
STACK_TRACE=kotlin.UninitializedPropertyAccessException: lateinit property basicKvStoreFactory has not been initialized
at fr.free.nrw.commons.upload.mediaDetails.UploadMediaPresenter.getBasicKvStoreFactory(UploadMediaPresenter.kt:54)
at fr.free.nrw.commons.upload.mediaDetails.UploadMediaPresenter.updateImageQualitiesJSON(UploadMediaPresenter.kt:366)
at fr.free.nrw.commons.upload.UploadPresenter.deletePictureAtIndex(UploadPresenter.kt:162)
at fr.free.nrw.commons.upload.UploadActivity.onThumbnailDeleted(UploadActivity.kt:776)
at fr.free.nrw.commons.upload.ThumbnailsAdapter$ViewHolder.bind$lambda$0(ThumbnailsAdapter.kt:72)
at fr.free.nrw.commons.upload.ThumbnailsAdapter$ViewHolder.$r8$lambda$h9qbE-GosgolnpmsfOlWqsny9oU(Unknown Source:0)
at fr.free.nrw.commons.upload.ThumbnailsAdapter$ViewHolder$$ExternalSyntheticLambda1.onClick(D8$$SyntheticClass:0)
at android.view.View.performClick(View.java:8081)
at android.view.View.performClickInternal(View.java:8058)
at android.view.View.-$$Nest$mperformClickInternal(Unknown Source:0)
at android.view.View$PerformClick.run(View.java:31517)
at android.os.Handler.handleCallback(Handler.java:991)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:232)
at android.os.Looper.loop(Looper.java:317)
at android.app.ActivityThread.main(ActivityThread.java:8934)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)

@rohit9625
Copy link
Contributor Author

Doesn't happen on my device. What are the exact steps you followed?
Apart from that, I encountered another IndexOutOfBoundsException when deleting images from a 4-image upload. Using notifyDataSetChanged instead of notifyItemRemoved fixed the issue. Would you please try deleting images from a 4-image upload @nicolas-raoul?
I'll push the changes once confirmed.

savsch and others added 6 commits January 18, 2025 03:19
…e either uploaded or marked. (commons-app#6095)

* feat: show the message "Congratulations, all pictures in this album have been either uploaded or marked as not for upload." in the custom picker when all images are either uploaded or marked.

* refactor: fix indentation in the code

* refactor: replace LiveData with StateFlow

* fix: fixed the bug that was causing one ImageFragment testcase to fail.

* fix: fixed the Congratulation message being shown for a brief moment while switching off the switch

* refactor: move hardcoded string to strings.xml.

* docs: add comment to clarify visibility logic in ImageFragment

---------

Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
…mons-app#6126)

* Rename Constants to Follow Kotlin Naming Conventions

>This PR refactors constant names in the project to adhere to Kotlin's UPPERCASE_SNAKE_CASE naming convention, improving code readability and maintaining consistency across the codebase.

>Renamed the following constants in LoginActivity:
>saveProgressDialog → SAVE_PROGRESS_DIALOG
>saveErrorMessage → SAVE_ERROR_MESSAGE
>saveUsername → SAVE_USERNAME
>savePassword → SAVE_PASSWORD

>Updated all references to these constants throughout the project.

* Update Project_Default.xml

* Refactor variable names to adhere to naming conventions

Renamed variables to use camel case:
-UPLOAD_COUNT_THRESHOLD → uploadCountThreshold
-REVERT_PERCENTAGE_FOR_MESSAGE → revertPercentageForMessage
-REVERT_SHARED_PREFERENCE → revertSharedPreference
-UPLOAD_SHARED_PREFERENCE → uploadSharedPreference

Renamed variables with uppercase initials to lowercase for alignment with Kotlin conventions:
-Latitude → latitude
-Longitude → longitude
-Accuracy → accuracy

Refactored the following variable names:
-NUMBER_OF_QUESTIONS → numberOfQuestions
-MULTIPLIER_TO_GET_PERCENTAGE → multiplierToGetPercentage

* Refactor Dialog View Initialization with Null-Safe Calls

This PR refactors the dialog setup code in CustomSelectorActivity to improve safety and readability by replacing explicit casts with null-safe generic calls for findViewById.

>Replaced explicit casting (as Button and as TextView) with the generic findViewById<T>() method for improved type safety.
>Added null-safety (?.) to avoid potential crashes if a view is not found in the dialog layout.

why changed:-
>Prevents runtime crashes caused by NullPointerException when a view is missing in the layout.

* Refactor Unit Test: Replace Unsafe Casting with Type-Safe Mocking for findViewById

>PR refactors the unit test for NearbyParentFragment by replacing unsafe casting in the findViewById mocking statements with type-safe

>Ensured all findViewById mocks now use a consistent, type-safe format (findViewById<View>(...)) to reduce verbosity and potential casting errors.

>Verified the functionality of prepareViewsForSheetPosition remains unchanged, ensuring no regression in test behavior.

* Update NearbyParentFragmentUnitTest.kt

* Refactor: Rename Constants to Follow CamelCase Naming Convention

>Updated all constant variable names to follow the camelCase naming convention, removing underscores in the middle or end.

>Ensured variable names remain descriptive and align with code readability best practices.

* Replace private val with const val for URL constants in QuizController

* Renaming the constant to use UPPER_SNAKE_CASE

* Renaming the constant to use UPPER_SNAKE_CASE

* Update Done

* **Refactor: Convert `minimumThresholdForSwipe` to a compile-time constant**

---------

Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
@nicolas-raoul
Copy link
Member

I just tried the latest commit of this branch, I get the same crash:

screen-20250119-073306.mp4

@rohit9625
Copy link
Contributor Author

I just tried the latest commit of this branch, I get the same crash:

Did you try uninstalling and then installing again? Please try using a different triplet of images.

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.

IndexOutOfBoundsException crash when removing last two images of multiupload
9 participants