From c37bcf8977a0653c93fd677e6c60160d52eabd0f Mon Sep 17 00:00:00 2001 From: RH Date: Fri, 29 Nov 2024 02:12:21 +1100 Subject: [PATCH] Prevent excessive calls to deleteBackward method which may cause crashes on Android (#2248) * Pass the number of characters to delete from a TextField * Include TextFieldEx.h reference in axmoil-ui.h * Remove logging calls used for testing * Update parameter name --- core/2d/TextFieldTTF.cpp | 28 +++++++++++++------ core/2d/TextFieldTTF.h | 2 +- core/base/IMEDelegate.h | 2 +- core/base/IMEDispatcher.cpp | 4 +-- core/base/IMEDispatcher.h | 2 +- core/platform/GLViewImpl.cpp | 2 +- .../src/org/axmol/lib/AxmolGLSurfaceView.java | 4 +-- .../java/src/org/axmol/lib/AxmolRenderer.java | 6 ++-- .../src/org/axmol/lib/TextInputWrapper.java | 18 ++++++------ .../jni/Java_org_axmol_lib_AxmolRenderer.cpp | 4 +-- core/platform/ios/InputView-ios.mm | 2 +- core/platform/winrt/InputEvent.cpp | 2 +- core/platform/winrt/Keyboard-winrt.cpp | 2 +- core/ui/UITextFieldEx.cpp | 26 +++++++++++------ core/ui/UITextFieldEx.h | 2 +- core/ui/axmol-ui.h | 1 + 16 files changed, 65 insertions(+), 42 deletions(-) diff --git a/core/2d/TextFieldTTF.cpp b/core/2d/TextFieldTTF.cpp index a5fbbb118b..971b259209 100644 --- a/core/2d/TextFieldTTF.cpp +++ b/core/2d/TextFieldTTF.cpp @@ -24,6 +24,8 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ****************************************************************************/ +#include + #include "2d/TextFieldTTF.h" #include "base/Director.h" @@ -310,7 +312,7 @@ void TextFieldTTF::insertText(const char* text, size_t len) detachWithIME(); } -void TextFieldTTF::deleteBackward() +void TextFieldTTF::deleteBackward(size_t numChars) { size_t len = _inputText.length(); if (!len) @@ -319,23 +321,33 @@ void TextFieldTTF::deleteBackward() return; } - // get the delete byte number - size_t deleteLen = 1; // default, erase 1 byte + // Length of characters to delete is based on input editor, but the actual + // length of the displayed text may be less + numChars = std::min(numChars, len); - while (0x80 == (0xC0 & _inputText.at(len - deleteLen))) + size_t totalDeleteLen = 0; + for (auto i = 0; i < numChars; ++i) { - ++deleteLen; + // get the delete byte number + size_t deleteLen = 1; // default, erase 1 byte + + // Calculate the actual number of bytes to delete for a specific character + while (0x80 == (0xC0 & _inputText.at(len - totalDeleteLen - deleteLen))) + { + ++deleteLen; + } + totalDeleteLen += deleteLen; } if (_delegate && - _delegate->onTextFieldDeleteBackward(this, _inputText.c_str() + len - deleteLen, static_cast(deleteLen))) + _delegate->onTextFieldDeleteBackward(this, _inputText.c_str() + len - totalDeleteLen, static_cast(totalDeleteLen))) { // delegate doesn't want to delete backwards return; } // if all text deleted, show placeholder string - if (len <= deleteLen) + if (len <= totalDeleteLen) { _inputText = ""; _charCount = 0; @@ -362,7 +374,7 @@ void TextFieldTTF::deleteBackward() } else { - std::string text(_inputText.c_str(), len - deleteLen); + std::string text(_inputText.c_str(), len - totalDeleteLen); setString(text); } } diff --git a/core/2d/TextFieldTTF.h b/core/2d/TextFieldTTF.h index 4a7d7a31fa..a66328d1f0 100644 --- a/core/2d/TextFieldTTF.h +++ b/core/2d/TextFieldTTF.h @@ -261,7 +261,7 @@ protected: virtual void didAttachWithIME() override; virtual void didDetachWithIME() override; virtual void insertText(const char* text, size_t len) override; - virtual void deleteBackward() override; + virtual void deleteBackward(size_t numChars) override; virtual std::string_view getContentText() override; virtual void controlKey(EventKeyboard::KeyCode keyCode) override; diff --git a/core/base/IMEDelegate.h b/core/base/IMEDelegate.h index 342767d432..2cbdfb07c3 100644 --- a/core/base/IMEDelegate.h +++ b/core/base/IMEDelegate.h @@ -124,7 +124,7 @@ protected: * @js NA * @lua NA */ - virtual void deleteBackward() {} + virtual void deleteBackward(size_t numChars) {} /** @brief Called by IMEDispatcher after the user press control key. diff --git a/core/base/IMEDispatcher.cpp b/core/base/IMEDispatcher.cpp index 929b1de351..c9090cb1fa 100644 --- a/core/base/IMEDispatcher.cpp +++ b/core/base/IMEDispatcher.cpp @@ -222,7 +222,7 @@ void IMEDispatcher::dispatchInsertText(const char* text, size_t len) } while (0); } -void IMEDispatcher::dispatchDeleteBackward() +void IMEDispatcher::dispatchDeleteBackward(int numChars) { do { @@ -231,7 +231,7 @@ void IMEDispatcher::dispatchDeleteBackward() // there is no delegate attached to IME AX_BREAK_IF(!_impl->_delegateWithIme); - _impl->_delegateWithIme->deleteBackward(); + _impl->_delegateWithIme->deleteBackward(numChars); } while (0); } diff --git a/core/base/IMEDispatcher.h b/core/base/IMEDispatcher.h index dbb2299826..f03e03508f 100644 --- a/core/base/IMEDispatcher.h +++ b/core/base/IMEDispatcher.h @@ -65,7 +65,7 @@ public: * @brief Dispatches the delete-backward operation. * @lua NA */ - void dispatchDeleteBackward(); + void dispatchDeleteBackward(int numChars); /** * @brief Dispatches the press control key operation. diff --git a/core/platform/GLViewImpl.cpp b/core/platform/GLViewImpl.cpp index 753d43e998..def1b0cdba 100644 --- a/core/platform/GLViewImpl.cpp +++ b/core/platform/GLViewImpl.cpp @@ -1220,7 +1220,7 @@ void GLViewImpl::onGLFWKeyCallback(GLFWwindow* /*window*/, int key, int /*scanco switch (g_keyCodeMap[key]) { case EventKeyboard::KeyCode::KEY_BACKSPACE: - IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(); + IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(1); break; case EventKeyboard::KeyCode::KEY_HOME: case EventKeyboard::KeyCode::KEY_KP_HOME: diff --git a/core/platform/android/java/src/org/axmol/lib/AxmolGLSurfaceView.java b/core/platform/android/java/src/org/axmol/lib/AxmolGLSurfaceView.java index 4b0457ed98..256d8e5aeb 100644 --- a/core/platform/android/java/src/org/axmol/lib/AxmolGLSurfaceView.java +++ b/core/platform/android/java/src/org/axmol/lib/AxmolGLSurfaceView.java @@ -458,11 +458,11 @@ public class AxmolGLSurfaceView extends GLSurfaceView { }); } - public void deleteBackward() { + public void deleteBackward(int numChars) { this.queueEvent(new Runnable() { @Override public void run() { - AxmolGLSurfaceView.this.mRenderer.handleDeleteBackward(); + AxmolGLSurfaceView.this.mRenderer.handleDeleteBackward(numChars); } }); } diff --git a/core/platform/android/java/src/org/axmol/lib/AxmolRenderer.java b/core/platform/android/java/src/org/axmol/lib/AxmolRenderer.java index 1d47c5011c..523df76dc3 100644 --- a/core/platform/android/java/src/org/axmol/lib/AxmolRenderer.java +++ b/core/platform/android/java/src/org/axmol/lib/AxmolRenderer.java @@ -176,15 +176,15 @@ public class AxmolRenderer implements GLSurfaceView.Renderer { } private static native void nativeInsertText(final String text); - private static native void nativeDeleteBackward(); + private static native void nativeDeleteBackward(final int numChars); private static native String nativeGetContentText(); public void handleInsertText(final String text) { AxmolRenderer.nativeInsertText(text); } - public void handleDeleteBackward() { - AxmolRenderer.nativeDeleteBackward(); + public void handleDeleteBackward(final int numChars) { + AxmolRenderer.nativeDeleteBackward(numChars); } public String getContentText() { diff --git a/core/platform/android/java/src/org/axmol/lib/TextInputWrapper.java b/core/platform/android/java/src/org/axmol/lib/TextInputWrapper.java index 10b23b8855..9d50feca54 100644 --- a/core/platform/android/java/src/org/axmol/lib/TextInputWrapper.java +++ b/core/platform/android/java/src/org/axmol/lib/TextInputWrapper.java @@ -90,8 +90,8 @@ public class TextInputWrapper implements TextWatcher, OnEditorActionListener { new_i += 1; } - for (; old_i < this.mText.length(); ++old_i) { - this.mGLSurfaceView.deleteBackward(); + if (old_i < this.mText.length()) { + this.mGLSurfaceView.deleteBackward(this.mText.length() - old_i); } int nModified = s.length() - new_i; @@ -118,13 +118,13 @@ public class TextInputWrapper implements TextWatcher, OnEditorActionListener { if (this.mGLSurfaceView.getEditText() == pTextView && this.isFullScreenEdit()) { // user press the action button, delete all old text and insert new text if (null != mOriginText) { - for (int i = this.mOriginText.length(); i > 0; i--) { - this.mGLSurfaceView.deleteBackward(); + if (!this.mOriginText.isEmpty()) { + this.mGLSurfaceView.deleteBackward(this.mOriginText.length()); } } - + String text = pTextView.getText().toString(); - + if (text != null) { /* If user input nothing, translate "\n" to engine. */ if ( text.compareTo("") == 0) { @@ -135,16 +135,16 @@ public class TextInputWrapper implements TextWatcher, OnEditorActionListener { text += '\n'; } } - + final String insertText = text; this.mGLSurfaceView.insertText(insertText); } - + if (pActionID == EditorInfo.IME_ACTION_DONE) { this.mGLSurfaceView.requestFocus(); } return false; } -} \ No newline at end of file +} diff --git a/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp b/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp index 9497e0f4cd..e90c2097d8 100644 --- a/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp +++ b/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp @@ -70,9 +70,9 @@ JNIEXPORT void JNICALL Java_org_axmol_lib_AxmolRenderer_nativeInsertText(JNIEnv* ax::IMEDispatcher::sharedDispatcher()->dispatchInsertText(pszText, strlen(pszText)); } -JNIEXPORT void JNICALL Java_org_axmol_lib_AxmolRenderer_nativeDeleteBackward(JNIEnv*, jclass) +JNIEXPORT void JNICALL Java_org_axmol_lib_AxmolRenderer_nativeDeleteBackward(JNIEnv*, jclass, jint numChars) { - ax::IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(); + ax::IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(numChars); } JNIEXPORT jstring JNICALL Java_org_axmol_lib_AxmolRenderer_nativeGetContentText(JNIEnv* env, jclass) diff --git a/core/platform/ios/InputView-ios.mm b/core/platform/ios/InputView-ios.mm index 4ff4a8c020..9f60e34bda 100644 --- a/core/platform/ios/InputView-ios.mm +++ b/core/platform/ios/InputView-ios.mm @@ -98,7 +98,7 @@ THE SOFTWARE. [self.myMarkedText release]; self.myMarkedText = nil; } - ax::IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(); + ax::IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(1); } - (void)insertText:(nonnull NSString*)text diff --git a/core/platform/winrt/InputEvent.cpp b/core/platform/winrt/InputEvent.cpp index be3db092ff..21782ae8d5 100644 --- a/core/platform/winrt/InputEvent.cpp +++ b/core/platform/winrt/InputEvent.cpp @@ -112,7 +112,7 @@ void KeyboardEvent::execute() //Director::getInstance()()->getKeypadDispatcher()->dispatchKeypadMSG(kTypeBackClicked); break; case AxmolKeyEvent::Back: - IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(); + IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(1); break; case AxmolKeyEvent::Enter: IMEDispatcher::sharedDispatcher()->dispatchInsertText("\n", 1); diff --git a/core/platform/winrt/Keyboard-winrt.cpp b/core/platform/winrt/Keyboard-winrt.cpp index f83552a159..9f662b6523 100644 --- a/core/platform/winrt/Keyboard-winrt.cpp +++ b/core/platform/winrt/Keyboard-winrt.cpp @@ -304,7 +304,7 @@ void KeyBoardWinRT::OnWinRTKeyboardEvent(WinRTKeyboardEventType type, KeyEventAr switch (keyCode) { case EventKeyboard::KeyCode::KEY_BACKSPACE: - IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(); + IMEDispatcher::sharedDispatcher()->dispatchDeleteBackward(1); break; case EventKeyboard::KeyCode::KEY_HOME: case EventKeyboard::KeyCode::KEY_KP_HOME: diff --git a/core/ui/UITextFieldEx.cpp b/core/ui/UITextFieldEx.cpp index 8bb85486ac..52873d107d 100644 --- a/core/ui/UITextFieldEx.cpp +++ b/core/ui/UITextFieldEx.cpp @@ -617,7 +617,7 @@ void TextFieldEx::insertText(const char* text, size_t len) this->closeIME(); } -void TextFieldEx::deleteBackward() +void TextFieldEx::deleteBackward(size_t numChars) { if (!_editable || !this->_enabled || 0 == _charCount) { @@ -634,23 +634,33 @@ void TextFieldEx::deleteBackward() return; } - // get the delete byte number - size_t deleteLen = 1; // default, erase 1 byte + // Length of characters to delete is based on input editor, but the actual + // length of the displayed text may be less + numChars = std::min(numChars, len); - while (0x80 == (0xC0 & _inputText.at(_insertPos - deleteLen))) + size_t totalDeleteLen = 0; + for (auto i = 0; i < numChars; ++i) { - ++deleteLen; + // get the delete byte number + size_t deleteLen = 1; // default, erase 1 byte + + // Calculate the actual number of bytes to delete for a specific character + while (0x80 == (0xC0 & _inputText.at(_insertPos - totalDeleteLen - deleteLen))) + { + ++deleteLen; + } + totalDeleteLen += deleteLen; } // if (_delegate && _delegate->onTextFieldDeleteBackward(this, _inputText.c_str() + len - deleteLen, // static_cast(deleteLen))) //{ - // // delegate doesn't wan't to delete backwards + // // delegate doesn't want to delete backwards // return; // } // if all text deleted, show placeholder string - if (len <= deleteLen) + if (len <= totalDeleteLen) { __moveCursor(-1); @@ -670,7 +680,7 @@ void TextFieldEx::deleteBackward() // set new input text std::string text = _inputText; // (inputText.c_str(), len - deleteLen); - text.erase(_insertPos - deleteLen, deleteLen); + text.erase(_insertPos - totalDeleteLen, totalDeleteLen); __moveCursor(-1); diff --git a/core/ui/UITextFieldEx.h b/core/ui/UITextFieldEx.h index 01700b73da..7cff67f008 100644 --- a/core/ui/UITextFieldEx.h +++ b/core/ui/UITextFieldEx.h @@ -140,7 +140,7 @@ protected: bool canAttachWithIME() override; bool canDetachWithIME() override; - void deleteBackward() override; + void deleteBackward(size_t numChars) override; std::string_view getContentText() override; void handleDeleteKeyEvent(); diff --git a/core/ui/axmol-ui.h b/core/ui/axmol-ui.h index d58491e762..2e6301d306 100644 --- a/core/ui/axmol-ui.h +++ b/core/ui/axmol-ui.h @@ -40,6 +40,7 @@ THE SOFTWARE. #include "ui/UIListView.h" #include "ui/UISlider.h" #include "ui/UITextField.h" +#include "ui/UITextFieldEx.h" #include "ui/UITextBMFont.h" #include "ui/UIPageView.h" #include "ui/UIHelper.h"