Amend 101363352c

Instead of just disabling MOUSE_MOTION on inset buttons, we now register
the coordinates where the latest mouse press happens, and if that's on
the inset button we are on when releasing, we toggle the inset.

Thus, minimal unintentional mouse movements don't lead to the button not
to respond.

src/Cursor.cpp

@@ -714,6 +714,7 @@ bool CursorData::confirmDeletion(bool const before) const
 Cursor::Cursor(BufferView & bv)
 	: CursorData(&bv.buffer()), bv_(&bv),
 	  x_target_(-1), textTargetOffset_(0),
+	  x_clickpos_(-1), y_clickpos_(-1),
 	  beforeDispatchPosX_(0), beforeDispatchPosY_(0)
 {}
 
@@ -1381,6 +1382,13 @@ void Cursor::updateTextTargetOffset()
 }
 
 
+void Cursor::setClickPos(int x, int y)
+{
+	x_clickpos_ = x;
+	y_clickpos_ = y;
+}
+
+
 bool Cursor::selHandle(bool selecting)
 {
 	//lyxerr << "Cursor::selHandle" << endl;

src/Cursor.h

@@ -380,6 +380,12 @@ public:
 	void setTargetX();
 	/// clear targetX, i.e. set it to -1
 	void clearTargetX();
+	/// return x position of latest mouse press or -1 if unset
+	int xClickPos() const { return x_clickpos_; }
+	/// return y position of latest mouse press or -1 if unset
+	int yClickPos() const { return y_clickpos_; }
+	/// register mouse press coordinates
+	void setClickPos(int x, int y);
 	/// set offset to actual position - targetX
 	void updateTextTargetOffset();
 	/// distance between actual and targeted position during last up/down in text
@@ -469,6 +475,9 @@ private:
 	int x_target_;
 	/// if a x_target cannot be hit exactly in a text, put the difference here
 	int textTargetOffset_;
+	/// Exact position of mouse click
+	int x_clickpos_;
+	int y_clickpos_;
 	/// position before dispatch started
 	DocIterator beforeDispatchCursor_;
 	/// cursor screen coordinates before dispatch started

src/Text.cpp

@@ -5223,6 +5223,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 		bvcur.setMark(false);
 		switch (cmd.button()) {
 		case mouse_button::button1:
+			bvcur.setClickPos(cmd.x(), cmd.y());
 			if (!bvcur.selection())
 				// Set the cursor
 				bvcur.resetAnchor();
@@ -5314,6 +5315,8 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 	case LFUN_MOUSE_RELEASE:
 		switch (cmd.button()) {
 		case mouse_button::button1:
+			// unregister last mouse press position
+			cur.bv().cursor().setClickPos(-1, -1);
 			// Cursor was set at LFUN_MOUSE_PRESS or LFUN_MOUSE_MOTION time.
 			// If there is a new selection, update persistent selection;
 			// otherwise, single click does not clear persistent selection

src/insets/Inset.cpp

@@ -383,26 +383,15 @@ bool Inset::showInsetDialog(BufferView * bv) const
 void Inset::doDispatch(Cursor & cur, FuncRequest &cmd)
 {
 	switch (cmd.action()) {
-	// FIXME: The LFUN_MOUSE_MOTION case is a potential fix for #12418, and maybe also
-	// #12820 and #12279. This needs to be tested in the pre-release. Also it might
-	// add slight regressions with inset selection (when selection starts on the button
-	// of an inset).
-	// After this has been tested (by Mac users primarily), this comment should be
-	// updated if the fix is kept or after it has been modified.
-	case LFUN_MOUSE_MOTION:
-		// Do not attempt to select while hovering the inset button only (#12418).
-		if (!cur.selection() && cmd.button() == mouse_button::button1
-		    && clickable(cur.bv(), cmd.x(), cmd.y())) {
-			cur.noScreenUpdate();
-			cur.dispatched();
-		} else
-			cur.undispatched();
-		break;
 
 	case LFUN_MOUSE_RELEASE:
-		// if the derived inset did not explicitly handle mouse_release,
+		// If the derived inset did not explicitly handle mouse_release,
-		// we assume we request the settings dialog
+		// we assume we request the settings dialog,
-		if (!cur.selection() && cmd.button() == mouse_button::button1
+		// except if we are about to select (MOUSE_MOTION that started
+		// outside the inset).
+		if ((!cur.selection() || covers(cur.bv(), cur.bv().cursor().xClickPos(),
+						cur.bv().cursor().yClickPos()))
+		    && cmd.button() == mouse_button::button1
 		    && clickable(cur.bv(), cmd.x(), cmd.y()) && hasSettings()) {
 			FuncRequest tmpcmd(LFUN_INSET_SETTINGS);
 			dispatch(cur, tmpcmd);

src/insets/InsetCollapsible.cpp

@@ -504,6 +504,8 @@ docstring const InsetCollapsible::getNewLabel(docstring const & l) const
 void InsetCollapsible::edit(Cursor & cur, bool front, EntryDirection entry_from)
 {
 	//lyxerr << "InsetCollapsible: edit left/right" << endl;
+	// We might have a selection if we moved the mouse on the button only
+	cur.clearSelection();
 	cur.push(*this);
 	InsetText::edit(cur, front, entry_from);
 }
@@ -577,9 +579,11 @@ void InsetCollapsible::doDispatch(Cursor & cur, FuncRequest & cmd)
 			cur.noScreenUpdate();
 			break;
 		}
-		// if we are selecting, we do not want to
+		// If we are selecting, we do not want to toggle the inset
-		// toggle the inset.
+		// except if the selection started at the inset button we're still on.
-		if (cur.selection())
+		// The latter addresses #12820.
+		if (cur.selection() && !clickable(cur.bv(), cur.bv().cursor().xClickPos(),
+						  cur.bv().cursor().yClickPos()))
 			break;
 		// Left button is clicked, the user asks to
 		// toggle the inset visual state.