Index: scribus/pageitem_textframe.cpp =================================================================== --- scribus/pageitem_textframe.cpp (revision 27773) +++ scribus/pageitem_textframe.cpp (working copy) @@ -4908,9 +4908,33 @@ // We have to use saved selection because mark removal may clear selection // so that we cannot get it back from itemText + int delStart = savedStart; + int delLen = savedStop - savedStart - marksNum; + + // If this deletion removes a paragraph separator, the paragraph that + // used to follow it gets merged into the current one. If that following + // paragraph starts with an automatic bullet/number mark (PP > numbering), + // the mark no longer sits at a paragraph start and becomes orphaned. + // Left alone, it would only be swept away later by the next layout pass + // (PageItem_TextFrame::updateBulletsNum()), *outside* of this undoable + // action - which desyncs the text length from the "START" position + // already recorded for this deletion and crashes StoryText::insertChars() + // on a later undo/redo. Remove the orphaned mark here, as part of the + // very same operation, so the text stays in sync with what undo expects. + if (delLen > 0) + { + int mergePos = delStart + delLen; + if ((mergePos < itemText.length()) + && (itemText.text(mergePos - 1) == SpecialChars::PARSEP) + && itemText.hasMark(mergePos, MARKBullNumType)) + { + ++delLen; + } + } + itemText.deselectAll(); - itemText.setCursorPosition(savedStart); - itemText.select(savedStart, savedStop - savedStart - marksNum); + itemText.setCursorPosition(delStart); + itemText.select(delStart, delLen); itemText.removeSelection(); HasSel = false; // m_Doc->updateFrameItems(); Index: scribus/pageitem.cpp =================================================================== --- scribus/pageitem.cpp (revision 27773) +++ scribus/pageitem.cpp (working copy) @@ -7086,6 +7086,52 @@ itemText.setStyle(pos, is->getNewState()); } +namespace { + +/** + * Automatic bullet/number marks (PageItem_TextFrame::updateBulletsNum(), + * called from layout()) are inserted into / removed from a frame's + * StoryText independently of the undo system, since they are a derived, + * regenerable annotation of the paragraph structure rather than user + * content. An offset captured by one of the DELETE_FRAMEPARA / DELETE_FRAMETEXT / + * INSERT_FRAMEPARA / INSERT_FRAMETEXT undo states can therefore become + * stale if such a mark is added or removed between the time the action + * was recorded and the time it is undone or redone. + * + * That staleness should not happen for a normal single undo step - the + * paragraph-merge/-split code paths that create or remove these marks are + * expected to keep their own recorded offsets in sync (see + * PageItem_TextFrame::deleteSelectedTextFromFrame()). This check is a + * last-resort safety net for any case that isn't: it never silently + * "succeeds" with a corrupted position, it reports the inconsistency so it + * can be tracked down, and it clamps into valid range so the action + * degrades to "insert/select at the nearest valid position" instead of + * crashing the assertion in StoryText. + */ +void clampUndoTextPosition(const StoryText &itemText, int &pos, const char *context) +{ + int len = itemText.length(); + if (pos <= len) + return; + qWarning("PageItem: stale undo/redo text offset %d > current length %d in %s " + "(automatic bullet/number marks changed since this action was " + "recorded); clamping to avoid a crash", pos, len, context); + pos = len; +} + +/// As clampUndoTextPosition(), but also keeps a companion selection length +/// (as used by the "select then delete" undo/redo branches) from running +/// past the end of the text once pos has been clamped. +void clampUndoTextSelection(const StoryText &itemText, int &pos, int &len, const char *context) +{ + clampUndoTextPosition(itemText, pos, context); + int available = itemText.length() - pos; + if (len > available) + len = available; +} + +} // namespace + void PageItem::restoreDeleteFrameText(SimpleState *ss, bool isUndo) { const auto *is = dynamic_cast *>(ss); @@ -7100,16 +7146,23 @@ itemText.deselectAll(); if (isUndo) { + clampUndoTextPosition(itemText, start, "restoreDeleteFrameText (undo)"); itemText.insertChars(start, text); itemText.applyCharStyle(start, text.length(), is->getItem()); itemText.setCursorPosition(start + text.length()); invalid = true; invalidateLayout(); + layout(); } else { - itemText.select(start, text.length()); - asTextFrame()->deleteSelectedTextFromFrame(); + int len = text.length(); + clampUndoTextSelection(itemText, start, len, "restoreDeleteFrameText (redo)"); + if (len > 0) + { + itemText.select(start, len); + asTextFrame()->deleteSelectedTextFromFrame(); + } } update(); } @@ -7127,16 +7180,23 @@ itemText.deselectAll(); if (isUndo) { + clampUndoTextPosition(itemText, start, "restoreDeleteFrameParagraph (undo)"); itemText.insertChars(start, SpecialChars::PARSEP); itemText.applyStyle(start, is->getItem()); itemText.setCursorPosition(start + 1); invalid = true; invalidateLayout(); + layout(); } else { - itemText.select(start, 1); - asTextFrame()->deleteSelectedTextFromFrame(); + int len = 1; + clampUndoTextSelection(itemText, start, len, "restoreDeleteFrameParagraph (redo)"); + if (len > 0) + { + itemText.select(start, len); + asTextFrame()->deleteSelectedTextFromFrame(); + } } update(); } @@ -7149,13 +7209,20 @@ itemText.deselectAll(); if (isUndo) { - itemText.select(start, text.length()); - asTextFrame()->deleteSelectedTextFromFrame(); + int len = text.length(); + clampUndoTextSelection(itemText, start, len, "restoreInsertFrameText (undo)"); + if (len > 0) + { + itemText.select(start, len); + asTextFrame()->deleteSelectedTextFromFrame(); + } } else { + clampUndoTextPosition(itemText, start, "restoreInsertFrameText (redo)"); itemText.insertChars(start, text); itemText.setCursorPosition(start + text.length()); + layout(); } // A row grow was folded into this insert (table-cell typing): restore the @@ -7186,16 +7253,23 @@ itemText.deselectAll(); if (isUndo) { - itemText.select(start, 1); - asTextFrame()->deleteSelectedTextFromFrame(); + int len = 1; + clampUndoTextSelection(itemText, start, len, "restoreInsertFrameParagraph (undo)"); + if (len > 0) + { + itemText.select(start, len); + asTextFrame()->deleteSelectedTextFromFrame(); + } itemText.applyStyle(start, is->getItem()); } else { + clampUndoTextPosition(itemText, start, "restoreInsertFrameParagraph (redo)"); itemText.insertChars(start, SpecialChars::PARSEP); itemText.setCursorPosition(start + 1); invalid = true; invalidateLayout(); + layout(); } update(); }