View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0013969 | Scribus | Undo/Redo | public | 2016-04-20 17:07 | 2026-08-17 13:33 |
| Reporter | Kunda | Assigned To | |||
| Priority | normal | Severity | crash | Reproducibility | sometimes |
| Status | confirmed | Resolution | open | ||
| Platform | MacOSX | OS | Yosemite | OS Version | 10.10.5 |
| Product Version | 1.5.2.svn | ||||
| Target Version | 1.5.5 | ||||
| Summary | 0013969: Crash when undoing numbered lists | ||||
| Description | See screencast http://recordit.co/Ws6MogaAP6 | ||||
| Steps To Reproduce | 1) open text box 2) type in 3 separate lines 3) highlight them and numerate them via PP 4) create a newline 5) delete newline so you end up on the line 3 Result: Numerated line number disappears 6) Undo/Redo Result: Crash | ||||
| Additional Information | OSX 10.10.5 1.5.2svn r21218 | ||||
| Tags | lists | ||||
| Attached Files | bt.txt (4,160 bytes)
Assertion failed: (pos <= length()), function insertChars, file /tmp/scribus-20160419-57395-1dq4631/scribus/text/storytext.cpp, line 478.
Process 68154 stopped
* thread #1: tid = 0xecdb7, 0x00007fff95800286 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
frame #0: 0x00007fff95800286 libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`__pthread_kill:
-> 0x7fff95800286 <+10>: jae 0x7fff95800290 ; <+20>
0x7fff95800288 <+12>: movq %rax, %rdi
0x7fff9580028b <+15>: jmp 0x7fff957fbc53 ; cerror_nocancel
0x7fff95800290 <+20>: retq
(lldb) bt
* thread #1: tid = 0xecdb7, 0x00007fff95800286 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
* frame #0: 0x00007fff95800286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff922779f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff957949ab libsystem_c.dylib`abort + 129
frame #3: 0x00007fff9575ca91 libsystem_c.dylib`__assert_rtn + 321
frame #4: 0x00000001007f276f Scribus`StoryText::insertChars(int, QString, bool) + 613
frame #5: 0x000000010018bfe0 Scribus`PageItem::restoreDeleteFrameText(SimpleState*, bool) + 228
frame #6: 0x0000000100186c2e Scribus`PageItem::restore(UndoState*, bool) + 1448
frame #7: 0x00000001004946c5 Scribus`TransactionState::undo() + 153
frame #8: 0x00000001004930e4 Scribus`UndoStack::undo(unsigned int, int) + 362
frame #9: 0x000000010048dc5d Scribus`UndoManager::undo(int) + 67
frame #10: 0x00000001038185bd QtCore`QMetaObject::activate(QObject*, int, int, void**) + 749
frame #11: 0x0000000100015247 Scribus`ScrAction::triggeredData(int) + 63
frame #12: 0x000000010037a52c Scribus`ScrAction::triggeredToTriggeredData() + 62
frame #13: 0x00000001038185bd QtCore`QMetaObject::activate(QObject*, int, int, void**) + 749
frame #14: 0x000000010233496f QtWidgets`QAction::activate(QAction::ActionEvent) + 303
frame #15: 0x00000001038185bd QtCore`QMetaObject::activate(QObject*, int, int, void**) + 749
frame #16: 0x00000001038116dc QtCore`QObject::event(QEvent*) + 156
frame #17: 0x000000010233db76 QtWidgets`QApplicationPrivate::notify_helper(QObject*, QEvent*) + 294
frame #18: 0x00000001023404c0 QtWidgets`QApplication::notify(QObject*, QEvent*) + 5648
frame #19: 0x00000001037e53f4 QtCore`QCoreApplication::notifyInternal2(QObject*, QEvent*) + 164
frame #20: 0x00000001037e6014 QtCore`QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) + 596
frame #21: 0x0000000104421bae libqcocoa.dylib`QCocoaEventDispatcherPrivate::processPostedEvents() + 190
frame #22: 0x0000000104422481 libqcocoa.dylib`QCocoaEventDispatcherPrivate::postedEventsSourceCallback(void*) + 33
frame #23: 0x00007fff98390a01 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
frame #24: 0x00007fff98382b8d CoreFoundation`__CFRunLoopDoSources0 + 269
frame #25: 0x00007fff983821bf CoreFoundation`__CFRunLoopRun + 927
frame #26: 0x00007fff98381bd8 CoreFoundation`CFRunLoopRunSpecific + 296
frame #27: 0x00007fff8cb5a56f HIToolbox`RunCurrentEventLoopInMode + 235
frame #28: 0x00007fff8cb5a1ee HIToolbox`ReceiveNextEventCommon + 179
frame #29: 0x00007fff8cb5a12b HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71
frame #30: 0x00007fff8d3da8ab AppKit`_DPSNextEvent + 978
frame #31: 0x00007fff8d3d9e58 AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346
frame #32: 0x00007fff8d3cfaf3 AppKit`-[NSApplication run] + 594
frame #33: 0x000000010442137f libqcocoa.dylib`QCocoaEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 2191
frame #34: 0x00000001037e17a1 QtCore`QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 401
frame #35: 0x00000001037e5a65 QtCore`QCoreApplication::exec() + 341
frame #36: 0x00000001007d7ad5 Scribus`mainApp(int, char**) + 90
frame #37: 0x00007fff9532b5c9 libdyld.dylib`start + 1
frame #38: 0x00007fff9532b5c9 libdyld.dylib`start + 1 | ||||
| Patch | No | ||||
|
|
Ben can you test on linux? thanks! |
|
|
Yes, it crashes Linux also (OpenSuSe Tumbleweed x86_64). Crash number 6 on the "undo delete"; never got a chance to do a redo. I also confirm that backing up to numbered line 3 by deleting unnumbered line 4 removes the line number on 3. Saving and reopening the file (Scribus restart too) with three numbered lines and one unnumbered one prior to deletion doesn't prevent the unnumbering or the crash on undo. Also observed a few other inelegancies: o It was quite difficult to include the first line in my selection when I dragged my mouse up and to the left from the end of the last line. It finally selected all three lines on the fifth try or so. I seem to need to start significantly below and to the right of the text (a running start). o When I added the numeration, the last 3 characters of the last line were no longer part of the selection. This was not just a visual quirk; hitting backspace left "?e 3" on the page. The line had read "Line 3" prior to numeration, the ? is explained next o At times the numeration was displayed as a question mark at the beginning of each line. This included when the numeration was first applied. Additional typing or numerating it a second time fixed the display. This could also be observed when the file was saved and reopened. Numeration displayed as question marks until I deleted the blank line under line 3. The resulting redraw replaced the question marks with numeration again (except on that last line, so we know saving the file doesn't help). o If I did a save with the text frame still selected (red outline) and then tried to quit, I received a warning of unsaved changes. Deselecting the text frame and saving again allowed me to exit without warning. The warning appears correct…sometimes. Sometimes the most current state of a selected text frame is saved, sometimes not. In either event, it always invokes a warning. Perhaps the save event could deselect items automatically without upsetting users? Hope this helps. |
|
|
The not selected characters is 0013912. The question marks are the refresh bug. Meta bug 0013075 |
|
|
Thanks for confirming guys. |
|
|
I could still reproduce it in r27773. Here how AI approached it. It fixed for me. numbered_para_merge_undo_crash_v2.00.patch (6,861 bytes)
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<ScItemState<CharStyle> *>(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();
}
|
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-04-20 17:07 | Kunda | New Issue | |
| 2016-04-20 17:07 | Kunda | Issue generated from: 0013728 | |
| 2016-04-20 17:07 | Kunda | Relationship added | related to 0013728 |
| 2016-04-20 17:10 | Kunda | Additional Information Updated | |
| 2016-04-20 17:10 | Kunda | File Added: bt.txt | |
| 2016-04-20 17:21 | Kunda | Target Version | => 1.5.2 |
| 2016-04-20 17:37 | FirasH | Relationship added | child of 0012500 |
| 2016-04-21 02:29 | Kunda | Note Added: 0040354 | |
| 2016-04-21 19:02 | Ben | Note Added: 0040372 | |
| 2016-04-21 19:02 | Ben | Note Edited: 0040372 | |
| 2016-04-21 19:20 |
|
Note Added: 0040373 | |
| 2016-04-21 23:38 | Kunda | Tag Attached: lists | |
| 2016-04-21 23:40 | Kunda | Note Added: 0040375 | |
| 2016-04-21 23:40 | Kunda | Status | new => confirmed |
| 2016-05-16 13:41 | cbradney | Target Version | 1.5.2 => 1.5.4.svn |
| 2016-12-08 21:32 | Kunda | Target Version | 1.5.4.svn => 1.5.5 |
| 2026-08-17 13:33 | qirat | Note Added: 0054220 | |
| 2026-08-17 13:33 | qirat | File Added: numbered_para_merge_undo_crash_v2.00.patch |