fix: Fixed ImHex crashing when using ctrl-backspace on empty file. (#2433)

Editor was attempting to delete non-existent chars which is UB. Fixed by checking before deleting. Also fixed was a problem created by having to press enter to change the search string which advanced the selection to the first match. In the next step one would expect that pressing enter on the replace field would replace the selected item but was replacing the item found after he first. This was fixed by always replacing the current selection first. If the replacement is the same as the searched term then replacing won't advance the cursor, but if they are different then the current match will no longer exist so it would search fora new one.
2026-03-28 07:47:03 -05:00 · 2025-09-05 02:28:11 -07:00
parent d62b64c27a
commit d8dd287cdf
10 changed files with 122 additions and 89 deletions
--- a/plugins/builtin/source/content/views/view_pattern_editor.cpp
+++ b/plugins/builtin/source/content/views/view_pattern_editor.cpp
@@ -904,7 +904,7 @@ namespace hex::plugin::builtin {
                    static std::string replaceWord;
                    static bool downArrowReplace = false;
                    static bool upArrowReplace = false;
-                    if (ImGui::InputTextWithHint("##replaceInputTextWidget", hint.c_str(), replaceWord, replaceFlags) || downArrowReplace || upArrowReplace) {
+                    if (ImGui::InputTextWithHint("###replaceInputTextWidget", hint.c_str(), replaceWord, replaceFlags) || downArrowReplace || upArrowReplace) {
                        findReplaceHandler->setReplaceWord(replaceWord);
                        historyInsert(m_replaceHistory, m_replaceHistorySize, m_replaceHistoryIndex, replaceWord);

@@ -972,7 +972,8 @@ namespace hex::plugin::builtin {

                if ((ImGui::IsKeyPressed(ImGuiKey_F3, false)) || downArrowFind || upArrowFind || enterPressedFind) {
                    historyInsert(m_findHistory, m_findHistorySize, m_findHistoryIndex, findWord);
-                    position = findReplaceHandler->findMatch(textEditor, !shift && !upArrowFind);
+                    i32 index = !shift && !upArrowFind ? 1 : -1;
+                    position = findReplaceHandler->findMatch(textEditor, index);
                    count = findReplaceHandler->getMatches().size();
                    updateCount = true;
                    downArrowFind = false;
@@ -2021,9 +2022,9 @@ namespace hex::plugin::builtin {
        ContentRegistry::UserInterface::addMenuItem({ "hex.builtin.menu.file", "hex.builtin.view.pattern_editor.menu.find_next" }, 1520, AllowWhileTyping + Keys::F3, [this] {
            if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
                ui::TextEditor::FindReplaceHandler *findReplaceHandler = editor->getFindReplaceHandler();
-                findReplaceHandler->findMatch(editor, true);
+                findReplaceHandler->findMatch(editor, 1);
            } else {
-                m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, true);
+                m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, 1);
            }
        }, [this] {
            if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
@@ -2039,9 +2040,9 @@ namespace hex::plugin::builtin {
        ContentRegistry::UserInterface::addMenuItem({ "hex.builtin.menu.file", "hex.builtin.view.pattern_editor.menu.find_previous" }, 1530, AllowWhileTyping + SHIFT + Keys::F3, [this] {
            if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
                ui::TextEditor::FindReplaceHandler *findReplaceHandler = editor->getFindReplaceHandler();
-                findReplaceHandler->findMatch(editor, false);
+                findReplaceHandler->findMatch(editor, -1);
            } else {
-                m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, false);
+                m_textEditor->getFindReplaceHandler()->findMatch(&*m_textEditor, -1);
            }
        }, [this] {
            if (auto editor = getEditorFromFocusedWindow(); editor != nullptr) {
--- a/plugins/ui/include/ui/text_editor.hpp
+++ b/plugins/ui/include/ui/text_editor.hpp
@@ -59,6 +59,12 @@ namespace hex::ui {
            Coordinates getSelectedColumns();
            bool isSingleLine();
            bool contains(Coordinates coordinates, int8_t endsInclusive=1);
+            bool operator==(const Selection &o) const {
+                return m_start == o.m_start && m_end == o.m_end;
+            }
+            bool operator!=(const Selection &o) const {
+                return m_start != o.m_start || m_end != o.m_end;
+            }
        };

        struct EditorState {
@@ -72,7 +78,7 @@ namespace hex::ui {
            typedef std::vector<EditorState> Matches;
            Matches &getMatches() { return m_matches; }
            bool findNext(TextEditor *editor);
-            u32 findMatch(TextEditor *editor, bool isNex);
+            u32 findMatch(TextEditor *editor, i32 index);
            bool replace(TextEditor *editor, bool right);
            bool replaceAll(TextEditor *editor);
            std::string &getFindWord() { return m_findWord; }
@@ -687,7 +693,6 @@ namespace hex::ui {
        bool m_raiseContextMenu = false;
        Coordinates m_focusAtCoords = {};
        bool m_updateFocus = false;
-        bool m_ensureCursorVisible = false;

        std::vector<std::string> m_clickableText;

--- a/plugins/ui/source/ui/text_editor/editor.cpp
+++ b/plugins/ui/source/ui/text_editor/editor.cpp
@@ -487,7 +487,6 @@ namespace hex::ui {
            return;

        auto pos = setCoordinates(m_state.m_cursorPosition);
-        //auto start = std::min(pos, m_state.m_selection.m_start);

        insertTextAt(pos, value);
        m_lines[pos.m_line].m_colorized = false;
--- a/plugins/ui/source/ui/text_editor/navigate.cpp
+++ b/plugins/ui/source/ui/text_editor/navigate.cpp
@@ -120,7 +120,7 @@ namespace hex::ui {
        auto oldPos = m_state.m_cursorPosition;


-        if (isEmpty() || oldPos.m_line >= (i64)m_lines.size())
+        if (isEmpty() || oldPos < Coordinates(0, 0))
            return;

        auto lindex = m_state.m_cursorPosition.m_line;
@@ -163,7 +163,7 @@ namespace hex::ui {

        auto oldPos = m_state.m_cursorPosition;

-        if (isEmpty() || oldPos.m_line >= (i64) m_lines.size())
+        if (isEmpty() || oldPos > setCoordinates(-1, -1))
            return;

        auto lindex = m_state.m_cursorPosition.m_line;
@@ -184,11 +184,9 @@ namespace hex::ui {
        }

        if (select) {
-            if (oldPos == m_interactiveSelection.m_end) {
-                m_interactiveSelection.m_end = Coordinates(m_state.m_cursorPosition);
-                if (m_interactiveSelection.m_end == Invalid)
-                    return;
-            } else if (oldPos == m_interactiveSelection.m_start)
+            if (oldPos == m_interactiveSelection.m_end)
+                m_interactiveSelection.m_end = m_state.m_cursorPosition;
+            else if (oldPos == m_interactiveSelection.m_start)
                m_interactiveSelection.m_start = m_state.m_cursorPosition;
            else {
                m_interactiveSelection.m_start = oldPos;
@@ -387,16 +385,17 @@ namespace hex::ui {
        auto &line = m_lines[at.m_line];
        auto charIndex = lineCoordinatesToIndex(at);

-        if (isWordChar(line.m_chars[charIndex])) {
-            while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1]))
-                --charIndex;
-        } else if (ispunct(line.m_chars[charIndex])) {
-            while (charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
-                --charIndex;
-        } else if (isspace(line.m_chars[charIndex])) {
-            while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
-                --charIndex;
+        bool found = false;
+        while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1])) {
+            found = true;
+            --charIndex;
        }
+        while (!found && charIndex > 0 && ispunct(line.m_chars[charIndex - 1])) {
+            found = true;
+            --charIndex;
+        }
+        while (!found && charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
+            --charIndex;
        return getCharacterCoordinates(at.m_line, charIndex);
    }

@@ -408,16 +407,18 @@ namespace hex::ui {
        auto &line = m_lines[at.m_line];
        auto charIndex = lineCoordinatesToIndex(at);

-        if (isWordChar(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && isWordChar(line.m_chars[charIndex]))
-                ++charIndex;
-        } else if (ispunct(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && ispunct(line.m_chars[charIndex]))
-                ++charIndex;
-        } else if (isspace(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
-                ++charIndex;
+        bool found = false;
+        while (charIndex < (i32) line.m_chars.size() && isWordChar(line.m_chars[charIndex])) {
+            found = true;
+            ++charIndex;
        }
+        while (!found && charIndex < (i32) line.m_chars.size() && ispunct(line.m_chars[charIndex])) {
+            found = true;
+            ++charIndex;
+        }
+        while (!found && charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
+            ++charIndex;
+
        return getCharacterCoordinates(at.m_line, charIndex);
    }

@@ -429,17 +430,16 @@ namespace hex::ui {
        auto &line = m_lines[at.m_line];
        auto charIndex = lineCoordinatesToIndex(at);

-        if (isspace(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
-                ++charIndex;
-        }
-        if (isWordChar(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && (isWordChar(line.m_chars[charIndex])))
-                ++charIndex;
-        } else if (ispunct(line.m_chars[charIndex])) {
-            while (charIndex < (i32) line.m_chars.size() && (ispunct(line.m_chars[charIndex])))
-                ++charIndex;
+        while (charIndex < (i32) line.m_chars.size() && isspace(line.m_chars[charIndex]))
+            ++charIndex;
+        bool found = false;
+        while (charIndex < (i32) line.m_chars.size() && (isWordChar(line.m_chars[charIndex]))) {
+            found = true;
+            ++charIndex;
        }
+        while (!found && charIndex < (i32) line.m_chars.size() && (ispunct(line.m_chars[charIndex])))
+            ++charIndex;
+
        return getCharacterCoordinates(at.m_line, charIndex);
    }

@@ -451,17 +451,17 @@ namespace hex::ui {
        auto &line = m_lines[at.m_line];
        auto charIndex = lineCoordinatesToIndex(at);

-        if (isspace(line.m_chars[charIndex - 1])) {
-            while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
-                --charIndex;
-        }
-        if (isWordChar(line.m_chars[charIndex - 1])) {
-            while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1]))
-                --charIndex;
-        } else if (ispunct(line.m_chars[charIndex - 1])) {
-            while (charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
-                --charIndex;
+        bool found = false;
+        while (charIndex > 0 && isspace(line.m_chars[charIndex - 1]))
+            --charIndex;
+
+        while (charIndex > 0 && isWordChar(line.m_chars[charIndex - 1])) {
+            found = true;
+            --charIndex;
        }
+        while (!found && charIndex > 0 && ispunct(line.m_chars[charIndex - 1]))
+            --charIndex;
+
        return getCharacterCoordinates(at.m_line, charIndex);
    }

--- a/plugins/ui/source/ui/text_editor/render.cpp
+++ b/plugins/ui/source/ui/text_editor/render.cpp
@@ -19,10 +19,10 @@ namespace hex::ui {
        m_topMarginChanged = true;
    }

-    void TextEditor::setFocusAtCoords(const Coordinates &coords, bool ensureVisible) {
+    void TextEditor::setFocusAtCoords(const Coordinates &coords, bool scrollToCursor) {
        m_focusAtCoords = coords;
        m_updateFocus = true;
-        m_ensureCursorVisible = ensureVisible;
+        m_scrollToCursor = scrollToCursor;
    }

    void TextEditor::clearErrorMarkers() {
@@ -199,11 +199,13 @@ namespace hex::ui {
        auto height = ImGui::GetWindowHeight() - m_topMargin - scrollBarSize - m_charAdvance.y;
        auto width = ImGui::GetWindowWidth() - windowPadding.x - scrollBarSize;

-        auto top = (i32) rint((m_topMargin > scrollY ? m_topMargin - scrollY : scrollY) / m_charAdvance.y);
-        auto bottom = top + (i32) rint(height / m_charAdvance.y);
+        auto topPixels = m_topMargin > scrollY ? m_topMargin - scrollY : scrollY;
+        auto top = (i32) rint(topPixels / m_charAdvance.y) + 1;
+        top -= (top >= (i32) m_lines.size());
+        auto bottom = (i32) rint((topPixels + height) / m_charAdvance.y);

        auto left = (i32) rint(scrollX / m_charAdvance.x);
-        auto right = left + (i32) rint(width / m_charAdvance.x);
+        auto right =  (i32) rint((scrollX + width) / m_charAdvance.x);

        auto pos = setCoordinates(m_state.m_cursorPosition);
        pos.m_column = (i32) rint(textDistanceToLineStart(pos) / m_charAdvance.x);
@@ -221,16 +223,24 @@ namespace hex::ui {
        }

        if (mScrollToCursorY) {
-            if (pos.m_line < top)
-                ImGui::SetScrollY(std::max(0.0f, pos.m_line * m_charAdvance.y));
-            if (pos.m_line > bottom)
+            if (pos.m_line < top) {
+                ImGui::SetScrollY(std::max(0.0f, (pos.m_line - 1) * m_charAdvance.y));
+                m_scrollToCursor = true;
+            }
+            if (pos.m_line > bottom) {
                ImGui::SetScrollY(std::max(0.0f, pos.m_line * m_charAdvance.y - height));
+                m_scrollToCursor = true;
+            }
        }
        if (mScrollToCursorX) {
-            if (pos.m_column < left)
+            if (pos.m_column < left) {
                ImGui::SetScrollX(std::max(0.0f, pos.m_column * m_charAdvance.x));
-            if (pos.m_column > right)
+                m_scrollToCursor = true;
+            }
+            if (pos.m_column > right) {
                ImGui::SetScrollX(std::max(0.0f, pos.m_column * m_charAdvance.x - width));
+                m_scrollToCursor = true;
+            }
        }
        m_oldTopMargin = m_topMargin;
    }
@@ -354,7 +364,7 @@ namespace hex::ui {
    void TextEditor::setFocus() {
        m_state.m_cursorPosition = m_focusAtCoords;
        resetCursorBlinkTime();
-        if (m_ensureCursorVisible)
+        if (m_scrollToCursor)
            ensureCursorVisible();

        if (!this->m_readOnly)
@@ -607,7 +617,7 @@ namespace hex::ui {
    ImVec2 TextEditor::calculateCharAdvance() const {
        /* Compute mCharAdvance regarding scaled font size (Ctrl + mouse wheel)*/
        const float fontSize = ImGui::GetFont()->CalcTextSizeA(ImGui::GetFontSize(), FLT_MAX, -1.0f, "#", nullptr, nullptr).x;
-        return ImVec2(fontSize, ImGui::GetTextLineHeightWithSpacing() * m_lineSpacing);
+        return ImVec2(fontSize, GImGui->FontSize * m_lineSpacing);
    }

    float TextEditor::textDistanceToLineStart(const Coordinates &aFrom) {
--- a/plugins/ui/source/ui/text_editor/support.cpp
+++ b/plugins/ui/source/ui/text_editor/support.cpp
@@ -4,15 +4,11 @@

 namespace hex::ui {
    bool TextEditor::Coordinates::operator==(const Coordinates &o) const {
-        return
-                m_line == o.m_line &&
-                m_column == o.m_column;
+        return m_line == o.m_line && m_column == o.m_column;
    }

    bool TextEditor::Coordinates::operator!=(const Coordinates &o) const {
-        return
-                m_line != o.m_line ||
-                m_column != o.m_column;
+        return m_line != o.m_line || m_column != o.m_column;
    }

    bool TextEditor::Coordinates::operator<(const Coordinates &o) const {
@@ -327,6 +323,8 @@ namespace hex::ui {
    void TextEditor::Line::erase(LineIterator begin, u64 count) {
        if (count == (u64) -1)
            count = m_chars.size() - (begin.m_charsIter - m_chars.begin());
+        else
+            count = std::min(count, (u64) (m_chars.size() - (begin.m_charsIter - m_chars.begin())));
        m_chars.erase(begin.m_charsIter, begin.m_charsIter + count);
        m_colors.erase(begin.m_colorsIter, begin.m_colorsIter + count);
        m_flags.erase(begin.m_flagsIter, begin.m_flagsIter + count);
@@ -557,6 +555,7 @@ namespace hex::ui {
                    io.WantCaptureMouse = true;
                    m_state.m_cursorPosition = m_interactiveSelection.m_end = screenPosToCoordinates(ImGui::GetMousePos());
                    setSelection(m_interactiveSelection);
+                    ensureCursorVisible();
                    resetBlinking = true;
                }
                if (resetBlinking)
@@ -575,7 +574,7 @@ namespace hex::ui {

 // The returned index is shown in the form
 //  'index of count' so 1 based
-    u32 TextEditor::FindReplaceHandler::findMatch(TextEditor *editor, bool isNext) {
+    u32 TextEditor::FindReplaceHandler::findMatch(TextEditor *editor, i32 index) {

        if (editor->m_textChanged || m_optionsChanged) {
            std::string findWord = getFindWord();
@@ -585,18 +584,34 @@ namespace hex::ui {
            findAllMatches(editor, findWord);
        }
        Coordinates targetPos = editor->m_state.m_cursorPosition;
-        if (editor->hasSelection())
-            targetPos = isNext ? editor->m_state.m_selection.m_end : editor->m_state.m_selection.m_start;
-
-        auto count = m_matches.size();
+        i32 count = m_matches.size();

        if (count == 0) {
            editor->setCursorPosition(targetPos);
            return 0;
        }

-        if (isNext) {
-            for (u32 i = 0; i < count; i++) {
+        if (editor->hasSelection()) {
+            i32 matchIndex = 0;
+            for (i32 i = 0; i < count; i++) {
+                if (m_matches[i].m_selection == editor->m_state.m_selection) {
+                    matchIndex = i;
+                    break;
+                }
+            }
+            if (matchIndex >= 0 && matchIndex <count) {
+                while (matchIndex + index < 0)
+                    index += count;
+                auto rem = (matchIndex + index) % count;
+                selectFound(editor, rem);
+                return rem + 1;
+            }
+        }
+        targetPos = index > 0 ? editor->m_state.m_selection.m_end : (index < 0 ? editor->m_state.m_selection.m_start :  editor->m_state.m_selection.m_start + Coordinates(0,1));
+
+
+        if (index >= 0) {
+            for (i32 i = 0; i < count; i++) {
                if (targetPos <= m_matches[i].m_selection.m_start) {
                    selectFound(editor, i);
                    return i + 1;
@@ -808,7 +823,8 @@ namespace hex::ui {
            } else
                editor->m_state.m_cursorPosition.m_column--;
        }
-        auto matchIndex = findMatch(editor, right);
+        i32 index = right ? 0 : -1;
+        auto matchIndex = findMatch(editor, index);
        if (matchIndex != 0) {
            UndoRecord u;
            u.m_before = editor->m_state;
--- a/plugins/ui/source/ui/text_editor/utf8.cpp
+++ b/plugins/ui/source/ui/text_editor/utf8.cpp
@@ -114,8 +114,10 @@ namespace hex::ui {
    TextEditor::Coordinates TextEditor::screenPosToCoordinates(const ImVec2 &position) {
        ImVec2 local = position - ImGui::GetCursorScreenPos();
        i32 lineNo = std::max(0, (i32) floor(local.y / m_charAdvance.y));
-        if (local.x < (m_leftMargin - 2_scaled) || lineNo >= (i32) m_lines.size() || m_lines[lineNo].empty())
-            return setCoordinates(std::min(lineNo, (i32) m_lines.size() - 1), 0);
+        if (lineNo >= (i32) m_lines.size())
+            return setCoordinates((i32) m_lines.size() - 1, -1);
+        else if (local.x < (m_leftMargin - 2_scaled) || m_lines[lineNo].empty())
+            return setCoordinates(lineNo, 0);
        std::string line = m_lines[lineNo].m_chars;
        local.x -= (m_leftMargin - 5_scaled);
        i32 count = 0;