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

IndexOutOfBoundsException crash when removing last two images of multiupload #6122

Open
nicolas-raoul opened this issue Jan 12, 2025 · 11 comments · May be fixed by #6124
Open

IndexOutOfBoundsException crash when removing last two images of multiupload #6122

nicolas-raoul opened this issue Jan 12, 2025 · 11 comments · May be fixed by #6124
Assignees
@rohit9625
Labels
bug

Comments

@nicolas-raoul
Copy link
Member

screen-20250112-142958.mp4 
STACK_TRACE=java.lang.IndexOutOfBoundsException: Index 2 out of bounds for length 2
at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:266)
at java.util.Objects.checkIndex(Objects.java:359)
at java.util.ArrayList.get(ArrayList.java:434)
at fr.free.nrw.commons.upload.UploadPresenter.deletePictureAtIndex(UploadPresenter.kt:148)
at fr.free.nrw.commons.upload.UploadActivity.onThumbnailDeleted(UploadActivity.java:786)
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

I'll take this issue 😀

@rohit9625 rohit9625 linked a pull request Jan 13, 2025 that will close this issue
Open
@rohit9625
Copy link
Contributor

Hey @nicolas-raoul, I've solved the issue and also found a strange behavior that you can see in the screencast below. Removing the last image removes all images except the first one.

Screen_recording_20250113_140703.mp4

@rohit9625
Copy link
Contributor

I should fix the above issue as well, right @nicolas-raoul?

@nicolas-raoul
Copy link
Member Author

Sorry I had forgotten about your comment above.

Feel free to either fix it or leave it as a separate issue, like you want. 🙂

@rohit9625
Copy link
Contributor

No problem, I am fixing this issue as well.

@rohit9625
Copy link
Contributor

I found that the image was not deleted but the top card was closed when deleting the last image due to this if-condition:

apps-android-commons/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.kt

Lines 149 to 153 in 3032270

if (index == uploadableFiles!!.size - 1) {
// If the next fragment to be shown is not one of the MediaDetailsFragment
// lets hide the top card so that it doesn't appear on the other fragments
view.showHideTopCard(false)
}

I don't understand the purpose of the above check but it seems like intended or maybe a mistake. What do you think @nicolas-raoul?

Removing above check fixed the issue:

Fix.mp4

@nicolas-raoul
Copy link
Member Author

I am not sure why the comment mentions MediaDetailsFragment, I believe MediaDetailsFragment was unrelated to upload.

Did you notice any bad side effect after removing it? If not, would you mind updating the pull request? Thanks!

@rohit9625
Copy link
Contributor

No, I didn't get any side-effects after removing that if-condition. I'm updating the pull-request :)

@rohit9625
Copy link
Contributor

rohit9625 commented Mar 9, 2025 •
edited
Loading

@nicolas-raoul, do we have the same Top Card to show image thumbnails on other screens?

@nicolas-raoul
Copy link
Member Author

do we have the same Top Card to show image thumbnails on other screens?

No, I am sure this top carousel is only used in the upload wizard.

@rohit9625
Copy link
Contributor

Alright @nicolas-raoul, I've pushed the changes. Please feel free to test the latest changes :

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

@rohit9625 rohit9625

Labels
bug
Projects
None yet
Milestone
No milestone
2 participants
@nicolas-raoul @rohit9625