r23626 by jghali - #15681: Crash when deleting text with footnotes

scribus-commit scribus-commit at lists.scribus.net
Mon Apr 20 10:35:16 UTC 2020


Author: jghali
Date: Mon Apr 20 10:35:15 2020
New Revision: 23626

URL: http://scribus.net/websvn/listing.php?repname=Scribus&sc=1&rev=23626
Log:
#15681: Crash when deleting text with footnotes

Modified:
    trunk/Scribus/scribus/pageitem_textframe.cpp
    trunk/Scribus/scribus/scribusdoc.cpp
    trunk/Scribus/scribus/text/sctext_shared.cpp
    trunk/Scribus/scribus/text/sctext_shared.h
    trunk/Scribus/scribus/text/storytext.cpp
    trunk/Scribus/scribus/text/storytext.h

Modified: trunk/Scribus/scribus/pageitem_textframe.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/pageitem_textframe.cpp
==============================================================================
--- trunk/Scribus/scribus/pageitem_textframe.cpp	(original)
+++ trunk/Scribus/scribus/pageitem_textframe.cpp	Mon Apr 20 10:35:15 2020
@@ -4536,6 +4536,8 @@
 	}
 	int start = itemText.startOfSelection();
 	int stop = itemText.endOfSelection();
+	int savedStart = itemText.startOfSelection();
+	int savedStop = itemText.endOfSelection();
 	int marksNum = 0;
 	if (UndoManager::undoEnabled()) 
 	{
@@ -4705,12 +4707,11 @@
 	else //remove marks without undo
 		marksNum = removeMarksFromText(false);
 
-	start = itemText.startOfSelection();
-	stop = itemText.endOfSelection();
-
-	itemText.setCursorPosition(start);
-	//for sure text is still selected
-	itemText.select(start, stop - start - marksNum);
+	// We have to use saved selection because mark removal may clear selection
+	// so that we cannot get it back from itemText
+	itemText.deselectAll();
+	itemText.setCursorPosition(savedStart);
+	itemText.select(savedStart, savedStop - savedStart - marksNum);
 	itemText.removeSelection();
 	HasSel = false;
 //	m_Doc->updateFrameItems();

Modified: trunk/Scribus/scribus/scribusdoc.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/scribusdoc.cpp
==============================================================================
--- trunk/Scribus/scribus/scribusdoc.cpp	(original)
+++ trunk/Scribus/scribus/scribusdoc.cpp	Mon Apr 20 10:35:15 2020
@@ -17260,10 +17260,11 @@
 			}
 		}
 	}
+
 	//remove mark references
-	for (int a=0; a < m_docMarksList.count(); ++a)
-	{
-		Mark* m = m_docMarksList.at(a);
+	for (int i = 0; i < m_docMarksList.count(); ++i)
+	{
+		Mark* m = m_docMarksList.at(i);
 		if (m == nullptr)
 			continue;
 		if (m->isType(MARK2MarkType))
@@ -17278,6 +17279,7 @@
 			}
 		}
 	}
+
 	//erase mark from marksMap
 	if (mrk->isUnique() || force)
 	{

Modified: trunk/Scribus/scribus/text/sctext_shared.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/text/sctext_shared.cpp
==============================================================================
--- trunk/Scribus/scribus/text/sctext_shared.cpp	(original)
+++ trunk/Scribus/scribus/text/sctext_shared.cpp	Mon Apr 20 10:35:15 2020
@@ -17,8 +17,7 @@
 #include "util.h"
 
 ScText_Shared::ScText_Shared(const StyleContext* pstyles) :
-	pstyleContext(nullptr),
-	refs(1), len(0), cursorPosition(0), marksCount(0), marksCountChanged(false)
+	pstyleContext(nullptr)
 {
 	pstyleContext.setDefaultStyle( & defaultStyle );
 	defaultStyle.setContext( pstyles );
@@ -32,7 +31,8 @@
 ScText_Shared::ScText_Shared(const ScText_Shared& other) :
 	defaultStyle(other.defaultStyle), 
 	pstyleContext(other.pstyleContext),
-	refs(1), len(0), cursorPosition(other.cursorPosition),
+	cursorPosition(other.cursorPosition),
+	selFirst(other.selFirst), selLast(other.selLast),
 	marksCount(other.marksCount), marksCountChanged(other.marksCountChanged),
 	trailingStyle(other.trailingStyle)
 {
@@ -65,6 +65,8 @@
 		delete this->takeFirst(); 
 	QList<ScText*>::clear();
 	cursorPosition = 0;
+	selFirst = 0;
+	selLast = -1;
 	if (marksCount > 0)
 		marksCountChanged = true;
 	marksCount = 0;
@@ -101,6 +103,8 @@
 		}
 		len = count();
 		cursorPosition = other.cursorPosition;
+		selFirst = other.selFirst;
+		selLast = other.selLast;
 		marksCount = other.marksCount;
 		marksCountChanged = other.marksCountChanged;
 		pstyleContext.invalidate();

Modified: trunk/Scribus/scribus/text/sctext_shared.h
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/text/sctext_shared.h
==============================================================================
--- trunk/Scribus/scribus/text/sctext_shared.h	(original)
+++ trunk/Scribus/scribus/text/sctext_shared.h	Mon Apr 20 10:35:15 2020
@@ -25,11 +25,13 @@
 
 	ParagraphStyle defaultStyle;
 	StyleContextProxy pstyleContext;
-	uint refs;
-	uint len;
-	uint cursorPosition;
-	uint marksCount;
-	bool marksCountChanged;
+	uint refs { 1 };
+	uint len { 0 };
+	uint cursorPosition { 0 };
+	int  selFirst { 0 };
+	int  selLast { -1 };
+	uint marksCount { 0 };
+	bool marksCountChanged { false };
 	ParagraphStyle trailingStyle;
 	CharStyle orphanedCharStyle;
 

Modified: trunk/Scribus/scribus/text/storytext.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/text/storytext.cpp
==============================================================================
--- trunk/Scribus/scribus/text/storytext.cpp	(original)
+++ trunk/Scribus/scribus/text/storytext.cpp	Mon Apr 20 10:35:15 2020
@@ -52,8 +52,8 @@
 	else
 		d = new ScText_Shared(nullptr);
 
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 	
 	m_shapedTextCache = new ShapedTextCache();
 	
@@ -65,8 +65,8 @@
 {
 	d = new ScText_Shared(nullptr);
 
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 	m_shapedTextCache = nullptr;
 }
 
@@ -81,8 +81,8 @@
 		m_doc->charStyles().connect(this, SLOT(invalidateAll()));
 	}
 	
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 	m_shapedTextCache = nullptr;
 
 	invalidateLayout();
@@ -198,8 +198,8 @@
 		m_doc->charStyles().connect(this, SLOT(invalidateAll()));
 	}
 	
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 
 	invalidateLayout();
 	return *this;
@@ -336,8 +336,8 @@
 
 void StoryText::clear()
 {
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 	
 	d->defaultStyle.erase();
 	d->trailingStyle.erase();
@@ -621,12 +621,12 @@
 		d->takeAt(i);
 		d->len--;
 		delete it;
-		// #9592 : adjust m_selFirst and m_selLast, those values have to be
+		// #9592 : adjust d->selFirst and d->selLast, those values have to be
 		// consistent in functions such as select()
-		if (i <= m_selLast)
-			--m_selLast;
-		if (i < m_selFirst)
-			--m_selFirst;
+		if (i <= d->selLast)
+			--d->selLast;
+		if (i < d->selFirst)
+			--d->selFirst;
 		if (static_cast<uint>(i + 1) <= d->cursorPosition && d->cursorPosition > 0)
 			d->cursorPosition -= 1;
 	}
@@ -636,10 +636,10 @@
 
 	d->len = d->count();
 	d->cursorPosition = qMin(d->cursorPosition, d->len);
-	if (m_selFirst > m_selLast)
-	{
-		m_selFirst =  0;
-		m_selLast  = -1;
+	if (d->selFirst > d->selLast)
+	{
+		d->selFirst =  0;
+		d->selLast  = -1;
 	}
 	invalidate(pos, length());
 }
@@ -1107,7 +1107,6 @@
 int StoryText::findMark(const Mark* mrk, int startPos) const
 {
 	assert(startPos >= 0);
-	assert(startPos < length());
 
 	int len = d->len;
 	for (int i = startPos; i < len; ++i)
@@ -1861,23 +1860,23 @@
 
 int StoryText::startOfSelection() const
 {
-	return m_selFirst <= m_selLast? m_selFirst : 0;
+	return d->selFirst <= d->selLast? d->selFirst : 0;
 }
 
 int StoryText::endOfSelection() const
 {
-	return m_selFirst <= m_selLast? m_selLast + 1 : -1;
+	return d->selFirst <= d->selLast? d->selLast + 1 : -1;
 }
 
 int StoryText::selectionLength() const
 {
 	//FIX ME - sometimes I saw values equal or greater than length of text
-	int last = m_selLast;
-	if (m_selFirst >= length())
+	int last = d->selLast;
+	if (d->selFirst >= length())
 		return 0;
-	if (m_selLast >= length())
+	if (d->selLast >= length())
 		last = length() -1;
-	return m_selFirst <= last? last - m_selFirst + 1 : 0;
+	return d->selFirst <= last? last - d->selFirst + 1 : 0;
 }
 
 bool StoryText::isSelected() const
@@ -1888,7 +1887,7 @@
 
 bool StoryText::selected(int pos) const
 {
-	return (m_selFirst <= pos && pos <= m_selLast) 
+	return (d->selFirst <= pos && pos <= d->selLast) 
 //	       || (pos >= 0 && pos < length() && const_cast<StoryText*>(this)->d->at(pos)->cselect)
 	;
 }
@@ -1935,7 +1934,7 @@
 	assert( pos >= 0 );
 	assert( pos + signed(len) <= length() );
 
-//	qDebug("old selection: %d - %d", m_selFirst, m_selLast);
+//	qDebug("old selection: %d - %d", d->selFirst, d->selLast);
 
 //	StoryText* that = const_cast<StoryText *>(this);
 //	for (int i=pos; i < pos+signed(len); ++i)
@@ -1944,45 +1943,45 @@
 	if (on) {
 		// extend if possible
 		if (selected(pos - 1))
-			m_selLast = qMax(m_selLast, pos + len - 1);
+			d->selLast = qMax(d->selLast, pos + len - 1);
 		else if (selected(pos + len))
-			m_selFirst = qMin(m_selFirst, pos);
+			d->selFirst = qMin(d->selFirst, pos);
 		else {
-			m_selFirst = pos;
-			m_selLast = pos + len - 1;
+			d->selFirst = pos;
+			d->selLast = pos + len - 1;
 		}
 	}
 	else {
-		if (pos <= m_selFirst && m_selLast < pos + signed(len))
+		if (pos <= d->selFirst && d->selLast < pos + signed(len))
 			deselectAll();
 		// shrink
 		else if (!selected(pos - 1) && selected(pos + len - 1))
-			m_selFirst = pos + len;
+			d->selFirst = pos + len;
 		else if (selected(pos) && !selected(pos + len))
-			m_selLast = pos - 1;
+			d->selLast = pos - 1;
 		else if (selected(pos) || selected(pos + len - 1))
 			// Grr, deselection splits selection
-			m_selLast = pos - 1;
+			d->selLast = pos - 1;
 	}
 
 	fixSurrogateSelection();
 	
-//	qDebug("new selection: %d - %d", m_selFirst, m_selLast);
+//	qDebug("new selection: %d - %d", d->selFirst, d->selLast);
 }
 
 void StoryText::extendSelection(int oldPos, int newPos)
 {
-	if (m_selFirst <= m_selLast)
+	if (d->selFirst <= d->selLast)
 	{
 		// have selection
-		if (m_selLast == oldPos - 1)
-		{
-			m_selLast = newPos - 1;
+		if (d->selLast == oldPos - 1)
+		{
+			d->selLast = newPos - 1;
 			return;
 		}
-		if (m_selFirst == oldPos)
-		{
-			m_selFirst = newPos;
+		if (d->selFirst == oldPos)
+		{
+			d->selFirst = newPos;
 			return;
 		}
 		// can't extend, fall through
@@ -1990,13 +1989,13 @@
 	// no previous selection
 	if (newPos > oldPos)
 	{
-		m_selFirst = oldPos;
-		m_selLast = newPos - 1;
+		d->selFirst = oldPos;
+		d->selLast = newPos - 1;
 	}
 	else
 	{
-		m_selFirst = newPos;
-		m_selLast = oldPos - 1;
+		d->selFirst = newPos;
+		d->selLast = oldPos - 1;
 	}
 
 	fixSurrogateSelection();
@@ -2005,10 +2004,10 @@
 
 void StoryText::fixSurrogateSelection()
 {
-	if (isLowSurrogate(m_selFirst) && isHighSurrogate(m_selFirst - 1))
-		m_selFirst -= 1;
-	if (isHighSurrogate(m_selLast) && isLowSurrogate(m_selLast + 1))
-		m_selLast += 1;
+	if (isLowSurrogate(d->selFirst) && isHighSurrogate(d->selFirst - 1))
+		d->selFirst -= 1;
+	if (isHighSurrogate(d->selLast) && isLowSurrogate(d->selLast + 1))
+		d->selLast += 1;
 }
 
 BreakIterator* StoryText::m_graphemeIterator = nullptr;
@@ -2087,8 +2086,8 @@
 		that->next();
 	}
 */
-	m_selFirst = 0;
-	m_selLast = length() - 1;
+	d->selFirst = 0;
+	d->selLast = length() - 1;
 }
 
 void StoryText::deselectAll()
@@ -2100,20 +2099,20 @@
 		that->next();
 	}
 */	
-	m_selFirst = 0;
-	m_selLast = -1;
+	d->selFirst = 0;
+	d->selLast = -1;
 }
 
 void StoryText::removeSelection()
 {
-//	qDebug("removeSelection: %d - %d", m_selFirst, m_selLast);
-	if (m_selFirst > m_selLast)
+//	qDebug("removeSelection: %d - %d", d->selFirst, d->selLast);
+	if (d->selFirst > d->selLast)
 		return;
 
-	assert( m_selFirst >= 0 );
-	assert( m_selLast < length() );
-
-	removeChars(m_selFirst, m_selLast - m_selFirst+1);
+	assert( d->selFirst >= 0 );
+	assert( d->selLast < length() );
+
+	removeChars(d->selFirst, d->selLast - d->selFirst+1);
 	deselectAll();
 }
 

Modified: trunk/Scribus/scribus/text/storytext.h
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23626&path=/trunk/Scribus/scribus/text/storytext.h
==============================================================================
--- trunk/Scribus/scribus/text/storytext.h	(original)
+++ trunk/Scribus/scribus/text/storytext.h	Mon Apr 20 10:35:15 2020
@@ -293,8 +293,7 @@
 	void fixSurrogateSelection();
 	
 private:
-	ScribusDoc * m_doc; 
-	int m_selFirst, m_selLast;
+	ScribusDoc * m_doc;
 	ShapedTextCache* m_shapedTextCache;
 	static BreakIterator* m_graphemeIterator;
 	static BreakIterator* m_wordIterator;




More information about the scribus-commit mailing list