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 crash on Ctrl+X with multiple carets in TextEdit #84901

Closed
wants to merge 1 commit into from

Conversation

Jordyfel
Copy link
Contributor

@Jordyfel Jordyfel commented Nov 14, 2023

Fix #81535 fix #83826

The crash was caused by this function occasionally leaving behind a caret with an invalid column index (larger than the column length), which caused a crash in an unrelated function. The fix is thus to clamp the caret index after cutting to the length of the new column.

However, the function has other problems. It's iteration is broken, since the backspace() call would remove carets during iteration, resulting in invalid index errors. So I changed the logic a little while trying to preserve the previous functionality. Also handled multiple carets accumulating on the same line after cutting, only one per line will remain now.

To reproduce a broken case fixed by this that doesn't cause a crash set two carets on one line, one on the line after it, and press Ctrl+X.

@Jordyfel Jordyfel requested a review from a team as a code owner November 14, 2023 17:20
@AThousandShips AThousandShips added bug topic:editor crash cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Nov 14, 2023
@AThousandShips AThousandShips added this to the 4.3 milestone Nov 14, 2023
@Jordyfel Jordyfel force-pushed the text-edit-crash branch 5 times, most recently from 4cb66fb to cbfa7fa Compare November 15, 2023 12:47
@kitbdev
Copy link
Contributor

kitbdev commented Dec 1, 2023

There is a crash when cutting and there is only one line.
Stacktrace:

godot.windows.editor.dev.x86_64.exe!CowData<TextEdit::Text::Line>::get(int p_index) Line 158 (c:\Projects\godotsrc\godot\core\templates\cowdata.h:158)
godot.windows.editor.dev.x86_64.exe!Vector<TextEdit::Text::Line>::operator[](int p_index) Line 96 (c:\Projects\godotsrc\godot\core\templates\vector.h:96)
godot.windows.editor.dev.x86_64.exe!TextEdit::Text::operator[](int p_line) Line 145 (c:\Projects\godotsrc\godot\scene\gui\text_edit.cpp:145)
godot.windows.editor.dev.x86_64.exe!TextEdit::_cut_internal(int p_caret) Line 6719 (c:\Projects\godotsrc\godot\scene\gui\text_edit.cpp:6719)
godot.windows.editor.dev.x86_64.exe!TextEdit::cut(int p_caret) Line 3720 (c:\Projects\godotsrc\godot\scene\gui\text_edit.cpp:3720)
godot.windows.editor.dev.x86_64.exe!TextEdit::gui_input(const Ref<InputEvent> & p_gui_input) Line 2136 (c:\Projects\godotsrc\godot\scene\gui\text_edit.cpp:2136)
godot.windows.editor.dev.x86_64.exe!CodeEdit::gui_input(const Ref<InputEvent> & p_gui_input) Line 568 (c:\Projects\godotsrc\godot\scene\gui\code_edit.cpp:568)
godot.windows.editor.dev.x86_64.exe!Control::_call_gui_input(const Ref<InputEvent> & p_event) Line 1811 (c:\Projects\godotsrc\godot\scene\gui\control.cpp:1811)
godot.windows.editor.dev.x86_64.exe!Viewport::_gui_input_event(Ref<InputEvent> p_event) Line 2236 (c:\Projects\godotsrc\godot\scene\main\viewport.cpp:2236)
godot.windows.editor.dev.x86_64.exe!Viewport::push_input(const Ref<InputEvent> & p_event, bool p_local_coords) Line 3327 (c:\Projects\godotsrc\godot\scene\main\viewport.cpp:3327)
godot.windows.editor.dev.x86_64.exe!Window::_window_input(const Ref<InputEvent> & p_ev) Line 1559 (c:\Projects\godotsrc\godot\scene\main\window.cpp:1559)
godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<Window,Ref<InputEvent> const &,0>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 303 (c:\Projects\godotsrc\godot\core\variant\binder_common.h:303)
godot.windows.editor.dev.x86_64.exe!call_with_variant_args<Window,Ref<InputEvent> const &>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 418 (c:\Projects\godotsrc\godot\core\variant\binder_common.h:418)
godot.windows.editor.dev.x86_64.exe!CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 99 (c:\Projects\godotsrc\godot\core\object\callable_method_pointer.h:99)
godot.windows.editor.dev.x86_64.exe!Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 58 (c:\Projects\godotsrc\godot\core\variant\callable.cpp:58)
godot.windows.editor.dev.x86_64.exe!Callable::call<Ref<InputEvent>>(Ref<InputEvent> <p_args_0>) Line 850 (c:\Projects\godotsrc\godot\core\variant\variant.h:850)
godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::_dispatch_input_event(const Ref<InputEvent> & p_event) Line 2716 (c:\Projects\godotsrc\godot\platform\windows\display_server_windows.cpp:2716)
godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::_dispatch_input_events(const Ref<InputEvent> & p_event) Line 2686 (c:\Projects\godotsrc\godot\platform\windows\display_server_windows.cpp:2686)
godot.windows.editor.dev.x86_64.exe!Input::_parse_input_event_impl(const Ref<InputEvent> & p_event, bool p_is_emulated) Line 731 (c:\Projects\godotsrc\godot\core\input\input.cpp:731)
godot.windows.editor.dev.x86_64.exe!Input::flush_buffered_events() Line 995 (c:\Projects\godotsrc\godot\core\input\input.cpp:995)

Otherwise this fixes the crash.

Another issue is cutting a folded line puts the caret on a hidden line.

Some minor issues that can be fixed in a separate PR:

  • The caret's visual column isn't the kept same on non fixed-width text, or on the last line.
  • Undoing a cut on an tabbed line moves the caret.
  • Cutting a wrapped line and undoing puts the caret on the first line wrap if there is only one caret.

And btw both indexes and indices are correct spellings.

@Jordyfel Jordyfel force-pushed the text-edit-crash branch 4 times, most recently from 6acb827 to 3c82fe9 Compare December 8, 2023 14:11
@Jordyfel
Copy link
Contributor Author

Jordyfel commented Dec 8, 2023

Thank you for the review!

Fixed the crash on only one line, handled hidden lines, and fixed the logic for preserving the visually perceived caret column.

@akien-mga akien-mga removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 9, 2024
@akien-mga
Copy link
Member

Superseded by #86978.

@akien-mga akien-mga closed this Apr 30, 2024
@akien-mga akien-mga added archived and removed cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Apr 30, 2024
@akien-mga akien-mga removed this from the 4.3 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants