Undo/Redo may be crash

[liyujiang-gzu]

Please consider making a Pull Request if you are capable of doing so.

App Version:
2020.2.2 Develop branch, means all version has the bug.

Affected Device(s):
ViVo x27 with Android 10

Describe the bug

    java.lang.ArrayIndexOutOfBoundsException: length=10; index=-1
        at java.util.ArrayList.get(ArrayList.java:439)
        at com.lightteam.editorkit.feature.undoredo.UndoStack.pop(UndoStack.kt:37)
        at com.lightteam.editorkit.internal.UndoRedoEditText.redo(UndoRedoEditText.kt:119)
        at com.lightteam.modpeide.ui.editor.fragments.EditorFragment.onRedoButton(EditorFragment.kt:616)
        at com.lightteam.modpeide.ui.editor.utils.ToolbarManager$bind$5.onClick(ToolbarManager.kt:129)
        at android.view.View.performClick(View.java:7187)
        at android.view.View.performClickInternal(View.java:7164)
        at android.view.View.access$3500(View.java:813)
        at android.view.View$PerformClick.run(View.java:27649)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:230)
        at android.app.ActivityThread.main(ActivityThread.java:7742)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:508)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1034)

    fun pop(): TextChange {
        // TODO: size must >= 1
        val item = stack[size - 1]
        stack.removeAt(size - 1)
        currentSize -= item.newText.length + item.oldText.length
        return item
    }

To Reproduce
Steps to reproduce the behavior:

1. Change content of Editor.
2. Quick click undo or redo button.
3. May be crash.

Expected behavior
No crash.

[massivemadness]

It's important to call undoStack.canUndo() before calling this method. Does it really reproducible or you just removed all canUndo checks? 🤔 I can't reproduce it on my device, everything works fine

[liyujiang-gzu]

It's important to call undoStack.canUndo() before calling this method. Does it really reproducible or you just removed all canUndo checks? 🤔 I can't reproduce it on my device, everything works fine

I fork develop branch, nothing to modified, but when you quickly double click Redo or Undo, it will crash. normal single click, it works fine.

[liyujiang-gzu]

I can not found canRedo() or canUndo() in call strack:
EditorFragment.kt

    override fun onUndoButton() {
        // TODO: The below line not found `canUndo()`
        binding.editor.undo()
    }

    override fun onRedoButton() {
        binding.editor.redo()
    }

[liyujiang-gzu]

It's important to call undoStack.canUndo() before calling this method. Does it really reproducible or you just removed all canUndo checks? 🤔 I can't reproduce it on my device, everything works fine

Reproduce step:

• Change text.
• 【Duoble click】 Undo button 【quickly】.
• Crash on my device.

[liyujiang-gzu]

When I change the text many times, I want quickly/fastly undo, but it crashed.😂😂😂
You can double click quickly/fastly to try again, may be reproduce.😹😹😹

[massivemadness]

@liyujiang-gzu Still cannot reproduce 😔
I really don't want to modify UndoStack.pop() because then it will have nullable return type.

Could you modify these methods and check it?

override fun onUndoButton() {
    if (binding.editor.canUndo()) {
        binding.editor.undo()
    }
}

override fun onRedoButton() {
    if (binding.editor.canRedo()) {
        binding.editor.redo()
    }
}

If everything will be fine, then I'll merge your PR into develop

[liyujiang-gzu]

@liyujiang-gzu Still cannot reproduce
I really don't want to modify UndoStack.pop() because then it will have nullable return type.

Could you modify these methods and check it?

override fun onUndoButton() {
    if (binding.editor.canUndo()) {
        binding.editor.undo()
    }
}

override fun onRedoButton() {
    if (binding.editor.canRedo()) {
        binding.editor.redo()
    }
}

If everything will be fine, then I'll merge your PR into develop

A great idea. I think the reason was:

If you click too fast, `android:clickable="@{viewModel.canRedo}"` was too late to change to false and then click again, so undo / redo cause crash.

I modified the above methods, It‘s works to fine, Clicking too fast will no longer cause a crash

[massivemadness]

Alright, I'll change it later, but if you want you can make pull request with these changes

[liyujiang-gzu]

I'll PR it later.