From: Stefan Schimanski <sts@lyx.org>
Date: Fri, 22 Feb 2008 20:45:18 +0000 (+0000)
Subject: * simplify notifyCursorLeaves logic: We now only call it from strategic
X-Git-Tag: 1.6.10~6155
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=8527a089035813a56a2963df56503216bff0eb04;p=features.git

* simplify notifyCursorLeaves logic: We now only call it from strategic
  places, i.e.
  - BufferView::setMouseCursor,
  - BufferView::mouseEventDispatch
  - LyXFunc::dispatch
  Before it was called from several dispatch handlers in the middle of the
  dispatch process. Because the cursor might get invalidated, this is
  not a good idea. We had plenty of crashes and odd behaviours because
  of this as nobody really understood those cases.

  This fixes (at least) #2069 and #4163.


git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@23140 a592a061-630c-0410-9148-cb99ea01b6c8
---

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 540ba866dd..33e157d584 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -1417,6 +1417,7 @@ void BufferView::mouseEventDispatch(FuncRequest const & cmd0)
 	// LFUN_FILE_OPEN generated by drag-and-drop.
 	FuncRequest cmd = cmd0;
 
+	Cursor old = cursor();
 	Cursor cur(*this);
 	cur.push(buffer_.inset());
 	cur.selection() = d->cursor_.selection();
@@ -1478,13 +1479,19 @@ void BufferView::mouseEventDispatch(FuncRequest const & cmd0)
 	if (!cur.result().dispatched())
 		cur.dispatch(cmd);
 
-	//Do we have a selection?
+	// Notify left insets
+	if (cur != old) {
+		old.fixIfBroken();
+		bool badcursor = notifyCursorLeaves(old, cur);
+		if (badcursor)
+			cursor().fixIfBroken();
+	}
+	
+	// Do we have a selection?
 	theSelection().haveSelection(cursor().selection());
 
 	// If the command has been dispatched,
-	if (cur.result().dispatched()
-		// an update is asked,
-		&& cur.result().update())
+	if (cur.result().dispatched() || cur.result().update())
 		processUpdateFlags(cur.result().update());
 }
 
@@ -1675,8 +1682,12 @@ bool BufferView::mouseSetCursor(Cursor & cur, bool select)
 	// Has the cursor just left the inset?
 	bool badcursor = false;
 	bool leftinset = (&d->cursor_.inset() != &cur.inset());
-	if (leftinset)
+	if (leftinset) {
+		d->cursor_.fixIfBroken();
 		badcursor = notifyCursorLeaves(d->cursor_, cur);
+		if (badcursor)
+			cur.fixIfBroken();
+	}
 
 	// FIXME: shift-mouse selection doesn't work well across insets.
 	bool do_selection = select && &d->cursor_.anchor().inset() == &cur.inset();
@@ -1689,24 +1700,7 @@ bool BufferView::mouseSetCursor(Cursor & cur, bool select)
 	if (!do_selection && !badcursor && d->cursor_.inTexted())
 		update |= checkDepm(cur, d->cursor_);
 
-	// if the cursor was in an empty script inset and the new
-	// position is in the nucleus of the inset, notifyCursorLeaves
-	// will kill the script inset itself. So we check all the
-	// elements of the cursor to make sure that they are correct.
-	// For an example, see bug 2933:
-	// http://bugzilla.lyx.org/show_bug.cgi?id=2933
-	// The code below could maybe be moved to a DocIterator method.
-	//lyxerr << "cur before " << cur << endl;
-	DocIterator dit = doc_iterator_begin(cur.inset());
-	dit.bottom() = cur.bottom();
-	size_t i = 1;
-	while (i < cur.depth() && dit.nextInset() == &cur[i].inset()) {
-		dit.push_back(cur[i]);
-		++i;
-	}
-	//lyxerr << "5 cur after" << dit <<endl;
-
-	d->cursor_.setCursor(dit);
+	d->cursor_.setCursor(cur);
 	d->cursor_.boundary(cur.boundary());
 	if (do_selection)
 		d->cursor_.setSelection();
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 405d72b4f0..b8e70680fd 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -377,8 +377,6 @@ void Cursor::pushBackward(Inset & p)
 bool Cursor::popBackward()
 {
 	BOOST_ASSERT(!empty());
-	//lyxerr << "Leaving inset from in front" << endl;
-	inset().notifyCursorLeaves(*this);
 	if (depth() == 1)
 		return false;
 	pop();
@@ -391,7 +389,6 @@ bool Cursor::popForward()
 	BOOST_ASSERT(!empty());
 	//lyxerr << "Leaving inset from in back" << endl;
 	const pos_type lp = (depth() > 1) ? (*this)[depth() - 2].lastpos() : 0;
-	inset().notifyCursorLeaves(*this);
 	if (depth() == 1)
 		return false;
 	pop();
@@ -1768,7 +1765,7 @@ bool Cursor::fixIfBroken()
 }
 
 
-bool notifyCursorLeaves(DocIterator const & old, Cursor & cur)
+bool notifyCursorLeaves(Cursor const & old, Cursor & cur)
 {
 	// find inset in common
 	size_type i;
@@ -1780,7 +1777,9 @@ bool notifyCursorLeaves(DocIterator const & old, Cursor & cur)
 	// notify everything on top of the common part in old cursor,
 	// but stop if the inset claims the cursor to be invalid now
 	for (; i < old.depth(); ++i) {
-		if (old[i].inset().notifyCursorLeaves(cur))
+		Cursor insetPos = old;
+		insetPos.cutOff(i);
+		if (old[i].inset().notifyCursorLeaves(insetPos, cur))
 			return true;
 	}
 	
diff --git a/src/Cursor.h b/src/Cursor.h
index 52220585b1..073880d9eb 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -398,7 +398,7 @@ public:
  * Sure that the cursor old is valid, i.e. all inset pointers
  * point to valid insets! Use Cursor::fixIfBroken if necessary.
  */
-bool notifyCursorLeaves(DocIterator const & old, Cursor & cur);
+bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 
 
 } // namespace lyx
diff --git a/src/LyXFunc.cpp b/src/LyXFunc.cpp
index f095a19a9d..13b5c091ff 100644
--- a/src/LyXFunc.cpp
+++ b/src/LyXFunc.cpp
@@ -1705,15 +1705,20 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
 			}
 
 			// Let the current Cursor dispatch its own actions.
+			Cursor old = view()->cursor();
 			view()->cursor().getPos(cursorPosBeforeDispatchX_,
 						cursorPosBeforeDispatchY_);
 			view()->cursor().dispatch(cmd);
-			updateFlags = view()->cursor().result().update();
-			if (!view()->cursor().result().dispatched()) {
-				// No update needed in this case (e.g. when reaching
-				// top of document.
-				updateFlags = Update::None;
+
+			// notify insets we just left
+			if (view()->cursor() != old) {
+				old.fixIfBroken();
+				bool badcursor = notifyCursorLeaves(old, view()->cursor());
+				if (badcursor)
+					view()->cursor().fixIfBroken();
 			}
+
+			updateFlags = view()->cursor().result().update();
 		}
 
 		if (lyx_view_ && lyx_view_->buffer()) {
diff --git a/src/Text3.cpp b/src/Text3.cpp
index b7f9619c96..b8e3ce0a20 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -590,10 +590,6 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 		bool up = cmd.action == LFUN_UP_SELECT || cmd.action == LFUN_UP;
 		bool const successful = cur.upDownInText(up, needsUpdate);
 		if (successful) {
-			// notify insets which were left and get their update flags 
-			notifyCursorLeaves(cur.beforeDispatchCursor(), cur);
-			cur.fixIfBroken();
-			
 			// redraw if you leave mathed (for the decorations)
 			needsUpdate |= cur.beforeDispatchCursor().inMathed();
 		} else
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index c3b72f4deb..c2d5a440c7 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -34,6 +34,7 @@ class Change;
 class Cursor;
 class CursorSlice;
 class Dimension;
+class DocIterator;
 class FuncRequest;
 class FuncStatus;
 class InsetCollapsable;
@@ -221,9 +222,8 @@ public:
 	/// number of columns in gridlike structures
 	virtual size_t ncols() const { return 0; }
 	/// is called when the cursor leaves this inset
-	/// returns true if cursor is now invalid. The cursor parameter
-	/// is _not_ necessarily pointing to the inset.
-	virtual bool notifyCursorLeaves(Cursor &) { return false; }
+	/// returns true if cursor is now invalid.
+	virtual bool notifyCursorLeaves(Cursor const &, Cursor &) { return false; }
 	/// is called when the mouse enter or leave this inset
 	/// return true if this inset needs repaint
 	virtual bool setMouseHover(bool) { return false; }
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index eb5a1a37e6..d3b7373bbe 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -414,7 +414,7 @@ void InsetMathHull::addPreview(graphics::PreviewLoader & ploader) const
 }
 
 
-bool InsetMathHull::notifyCursorLeaves(Cursor & cur)
+bool InsetMathHull::notifyCursorLeaves(Cursor const & /*old*/, Cursor & cur)
 {
 	if (RenderPreview::status() == LyXRC::PREVIEW_ON) {
 		Buffer const & buffer = cur.buffer();
@@ -1046,7 +1046,6 @@ void InsetMathHull::doDispatch(Cursor & cur, FuncRequest & cmd)
 	case LFUN_FINISHED_LEFT:
 		//lyxerr << "action: " << cmd.action << endl;
 		InsetMathGrid::doDispatch(cur, cmd);
-		notifyCursorLeaves(cur);
 		cur.undispatched();
 		break;
 
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index a7a3701da8..6904d5847a 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -116,7 +116,7 @@ public:
 	virtual void textString(Buffer const &, odocstream &) const;
 
 	/// get notification when the cursor leaves this inset
-	bool notifyCursorLeaves(Cursor & cur);
+	bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 	///
 	//bool insetAllowed(InsetCode code) const;
 	///
diff --git a/src/mathed/InsetMathNest.cpp b/src/mathed/InsetMathNest.cpp
index 14ec77cfd0..f1ab2cdc82 100644
--- a/src/mathed/InsetMathNest.cpp
+++ b/src/mathed/InsetMathNest.cpp
@@ -371,7 +371,7 @@ bool InsetMathNest::setMouseHover(bool mouse_hover)
 }
 
 
-bool InsetMathNest::notifyCursorLeaves(Cursor & /*cur*/)
+bool InsetMathNest::notifyCursorLeaves(Cursor const & /*old*/, Cursor & /*cur*/)
 {
 	// FIXME: look here
 #if 0
@@ -584,12 +584,8 @@ void InsetMathNest::doDispatch(Cursor & cur, FuncRequest & cmd)
 		// go up/down
 		bool up = cmd.action == LFUN_UP || cmd.action == LFUN_UP_SELECT;
 		bool successful = cur.upDownInMath(up);
-		if (successful) {
-			// notify left insets and give them chance to set update flags
-			lyx::notifyCursorLeaves(cur.beforeDispatchCursor(), cur);
-			cur.fixIfBroken();
+		if (successful)
 			break;
-		}
 		
 		if (cur.fixIfBroken())
 			// FIXME: Something bad happened. We pass the corrected Cursor
diff --git a/src/mathed/InsetMathNest.h b/src/mathed/InsetMathNest.h
index d164fe7525..d3fc72cc8b 100644
--- a/src/mathed/InsetMathNest.h
+++ b/src/mathed/InsetMathNest.h
@@ -97,7 +97,7 @@ public:
 	/// access to the lock
 	void lock(bool);
 	/// get notification when the cursor leaves this inset
-	bool notifyCursorLeaves(Cursor & cur);
+	bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 
 	/// direct access to the cell.
 	/// inlined because shows in profile.
diff --git a/src/mathed/InsetMathScript.cpp b/src/mathed/InsetMathScript.cpp
index c146e071b1..b1cc23eec0 100644
--- a/src/mathed/InsetMathScript.cpp
+++ b/src/mathed/InsetMathScript.cpp
@@ -660,9 +660,9 @@ void InsetMathScript::infoize2(odocstream & os) const
 }
 
 
-bool InsetMathScript::notifyCursorLeaves(Cursor & cur)
+bool InsetMathScript::notifyCursorLeaves(Cursor const & old, Cursor & cur)
 {
-	InsetMathNest::notifyCursorLeaves(cur);
+	InsetMathNest::notifyCursorLeaves(old, cur);
 
 	//LYXERR0("InsetMathScript::notifyCursorLeaves: 1 " << cur);
 
@@ -690,14 +690,13 @@ bool InsetMathScript::notifyCursorLeaves(Cursor & cur)
 	if ((nargs() == 2 && cell(1).empty())
 	    || (nargs() == 3 && cell(1).empty() && cell(2).empty())) {
 		// Make undo step. We cannot use cur for this because
-		// it does not necessarily point to us. The BufferView
-		// cursor though should do.
-		int scriptSlice
-		= cur.bv().cursor().find(this);
+		// it does not necessarily point to us anymore. But we
+		// should be on top of the cursor old.
+		Cursor insetCur = old;
+		int scriptSlice	= insetCur.find(this);
 		BOOST_ASSERT(scriptSlice != -1);
-		Cursor & bvCur = cur.bv().cursor();
-		bvCur.cutOff(scriptSlice);
-		bvCur.recordUndoInset();
+		insetCur.cutOff(scriptSlice);
+		insetCur.recordUndoInset();
 
 		// Let the script inset commit suicide. This is
 		// modelled on Cursor.pullArg(), but tries not to
@@ -705,12 +704,9 @@ bool InsetMathScript::notifyCursorLeaves(Cursor & cur)
 		// cur (since the top slice will be deleted
 		// afterwards)
 		MathData ar = cell(0);
-		bvCur.pop();
-		bvCur.cell().erase(bvCur.pos());
-		bvCur.cell().insert(bvCur.pos(), ar);
-
-		// put cursor behind
-		bvCur.pos() += ar.size();
+		insetCur.pop();
+		insetCur.cell().erase(insetCur.pos());
+		insetCur.cell().insert(insetCur.pos(), ar);
 
 		// redraw
 		cur.updateFlags(cur.disp_.update() | Update::SinglePar);
diff --git a/src/mathed/InsetMathScript.h b/src/mathed/InsetMathScript.h
index e851e2b371..a22cae95f4 100644
--- a/src/mathed/InsetMathScript.h
+++ b/src/mathed/InsetMathScript.h
@@ -129,7 +129,7 @@ private:
 	/// where do we have to draw the scripts?
 	bool hasLimits() const;
 	/// clean up empty cells and return true if a cell has been deleted.
-	bool notifyCursorLeaves(Cursor & cur);
+	bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 
 	/// possible subscript (index 0) and superscript (index 1)
 	bool cell_1_is_up_;
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 821ba2326d..f5f16ef358 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -606,10 +606,10 @@ bool MathMacro::idxLast(Cursor & cur) const
 }
 
 
-bool MathMacro::notifyCursorLeaves(Cursor & cur)
+bool MathMacro::notifyCursorLeaves(Cursor const & old, Cursor & cur)
 {
 	cur.updateFlags(Update::Force);
-	return InsetMathNest::notifyCursorLeaves(cur);
+	return InsetMathNest::notifyCursorLeaves(old, cur);
 }
 
 
diff --git a/src/mathed/MathMacro.h b/src/mathed/MathMacro.h
index e0567a6468..8a10a45300 100644
--- a/src/mathed/MathMacro.h
+++ b/src/mathed/MathMacro.h
@@ -56,7 +56,7 @@ public:
 	bool idxLast(Cursor &) const;
 
 	///
-	virtual bool notifyCursorLeaves(Cursor &);
+	virtual bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 	
 	/// Remove cell (starting from 0)
 	void removeArgument(pos_type pos);
diff --git a/src/mathed/MathMacroTemplate.cpp b/src/mathed/MathMacroTemplate.cpp
index ff0bc99c6d..70fc28b5dd 100644
--- a/src/mathed/MathMacroTemplate.cpp
+++ b/src/mathed/MathMacroTemplate.cpp
@@ -538,11 +538,11 @@ void MathMacroTemplate::edit(Cursor & cur, bool front, EntryDirection entry_from
 }
 
 
-bool MathMacroTemplate::notifyCursorLeaves(Cursor & cur)
+bool MathMacroTemplate::notifyCursorLeaves(Cursor const & old, Cursor & cur)
 {
 	updateLook();
 	cur.updateFlags(Update::Force);
-	return InsetMathNest::notifyCursorLeaves(cur);
+	return InsetMathNest::notifyCursorLeaves(old, cur);
 }
 
 
diff --git a/src/mathed/MathMacroTemplate.h b/src/mathed/MathMacroTemplate.h
index 6f2fa1d3b7..f161167382 100644
--- a/src/mathed/MathMacroTemplate.h
+++ b/src/mathed/MathMacroTemplate.h
@@ -40,7 +40,7 @@ public:
 	///
 	void edit(Cursor & cur, bool front, EntryDirection entry_from);
 	///
-	bool notifyCursorLeaves(Cursor & cur);
+	bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
 	///
 	void read(Buffer const &, Lexer & lex);
 	///