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

Do not iterate in while when history cursor is outside of list range #1065

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

planarvoid
Copy link
Contributor

@planarvoid planarvoid commented Oct 24, 2023

Fix

This PR fixes the following crash. We shouldn't iterate the list if the cursor points out of it.

IndexOutOfBoundsException: Index: 10, Size: 0
    at java.util.LinkedList.checkElementIndex(LinkedList.java:559)
    at java.util.LinkedList.remove(LinkedList.java:529)
    at org.wordpress.aztec.History.doHandleHistory(History.kt:65)
    at org.wordpress.aztec.History$HistoryRunnable.run(History.kt:0)
    at android.os.Handler.handleCallback(Handler.java:938)

Test

  1. I think this is a race condition so there is no way to test this

Review

@khaykov

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@planarvoid planarvoid added the bug label Oct 24, 2023
@planarvoid planarvoid requested a review from khaykov October 24, 2023 14:07
@khaykov khaykov self-assigned this Nov 6, 2023
Copy link
Member

@khaykov khaykov left a comment

Choose a reason for hiding this comment

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

I looked into reproducing this one some time ago, but no luck. Thanks for handling it!

@khaykov khaykov merged commit c36ac5f into trunk Nov 6, 2023
14 checks passed
@khaykov khaykov deleted the fix/only-remove-actual-history-items branch November 6, 2023 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants