Make TextMetrics::editXY more robust

This fixes a crash in examples/fa/splash.lyx when selecting text
representing menu entries. This happens because menu names are in LTR
English, while the inset itself is in RTL.

The problem is that the current code relies on the fact that
 1. getColumnNearX and checkInsetHit share the same idea about cursor
    position.
 2. pos and pos + 1 are in general consecutive on screen.

It seems that 1. is wrong here (for reasons I did not try to
understand); the second assumption is definitely false with
bi-directional text. This makes editXY very fragile.

The new code should be more robust in this respect. The logic is:
 * if checkInsetHit finds an inset, use its position,
 * otherwise, ask getColumnNearX for the cursor position.

Fixes: #9142

src/TextMetrics.cpp

@@ -1492,37 +1492,30 @@ Inset * TextMetrics::editXY(Cursor & cur, int x, int y,
 	int yy = y; // is modified by getPitAndRowNearY
 	Row const & row = getPitAndRowNearY(yy, pit, assert_in_view, up);
 
-	bool bound = false; // is modified by getColumnNearX
-	int xx = x; // is modified by getColumnNearX
-	pos_type const pos = row.pos()
-		+ getColumnNearX(pit, row, xx, bound);
 	cur.pit() = pit;
-	cur.pos() = pos;
-	cur.boundary(bound);
-	cur.setTargetX(x);
 
-	// try to descend into nested insets
-	Inset * inset = checkInsetHit(x, yy);
-	//lyxerr << "inset " << inset << " hit at x: " << x << " y: " << y << endl;
-	if (!inset) {
+	// Do we cover an inset?
+	InsetList::InsetTable * it = checkInsetHit(pit, x, yy);
+
+	if (!it) {
+		// No inset, set position in the text
+		bool bound = false; // is modified by getColumnNearX
+		int xx = x; // is modified by getColumnNearX
+		cur.pos() = row.pos()
+			+ getColumnNearX(pit, row, xx, bound);
+		cur.boundary(bound);
 		cur.setCurrentFont();
+		cur.setTargetX(xx);
 		return 0;
 	}
 
-	ParagraphList const & pars = text_->paragraphs();
-	Inset const * inset_before = pos ? pars[pit].getInset(pos - 1) : 0;
+	Inset * inset = it->inset;
+	//lyxerr << "inset " << inset << " hit at x: " << x << " y: " << y << endl;
 
-	// This should be just before or just behind the
-	// cursor position set above.
-	LASSERT(inset == inset_before
-		|| inset == pars[pit].getInset(pos), return 0);
-
-	// Make sure the cursor points to the position before
-	// this inset.
-	if (inset == inset_before) {
-		--cur.pos();
-		cur.boundary(false);
-	}
+	// Set position in front of inset
+	cur.pos() = it->pos;
+	cur.boundary(false);
+	cur.setTargetX(x);
 
 	// Try to descend recursively inside the inset.
 	inset = inset->editXY(cur, x, yy);
@@ -1571,11 +1564,8 @@ void TextMetrics::setCursorFromCoordinates(Cursor & cur, int const x, int const
 
 
 //takes screen x,y coordinates
-Inset * TextMetrics::checkInsetHit(int x, int y)
+InsetList::InsetTable * TextMetrics::checkInsetHit(pit_type pit, int x, int y)
 {
-	pit_type pit = getPitNearY(y);
-	LASSERT(pit != -1, return 0);
-
 	Paragraph const & par = text_->paragraphs()[pit];
 	ParagraphMetrics const & pm = par_metrics_[pit];
 
@@ -1604,7 +1594,7 @@ Inset * TextMetrics::checkInsetHit(int x, int y)
 			&& y >= p.y_ - dim.asc
 			&& y <= p.y_ + dim.des) {
 			LYXERR(Debug::DEBUG, "Hit inset: " << inset);
-			return inset;
+			return const_cast<InsetList::InsetTable *>(&(*iit));
 		}
 	}
 
@@ -1613,6 +1603,20 @@ Inset * TextMetrics::checkInsetHit(int x, int y)
 }
 
 
+//takes screen x,y coordinates
+Inset * TextMetrics::checkInsetHit(int x, int y)
+{
+	pit_type const pit = getPitNearY(y);
+	LASSERT(pit != -1, return 0);
+	InsetList::InsetTable * it = checkInsetHit(pit, x, y);
+
+	if (!it)
+		return 0;
+
+	return it->inset;
+}
+
+
 int TextMetrics::cursorX(CursorSlice const & sl,
 		bool boundary) const
 {

src/TextMetrics.h

@@ -15,6 +15,7 @@
 #define TEXT_METRICS_H
 
 #include "Font.h"
+#include "InsetList.h"
 #include "ParagraphMetrics.h"
 
 #include "support/types.h"
@@ -36,7 +37,7 @@ public:
 	TextMetrics() : text_(0) {}
 	/// The only useful constructor.
 	TextMetrics(BufferView *, Text *);
-	
+
 	///
 	bool contains(pit_type pit) const;
 	///
@@ -155,6 +156,10 @@ private:
 		pos_type const end
 		) const;
 
+	// Helper function for the other checkInsetHit method.
+	InsetList::InsetTable * checkInsetHit(pit_type pit, int x, int y);
+
+
 // Temporary public:
 public:
 	/// returns the column near the specified x-coordinate of the row.

status.21x

@@ -57,6 +57,8 @@ What's new
 
 * DOCUMENT INPUT/OUTPUT
 
+- Fix assertion when selecting in document with bi-directional text (bug 9142).
+
 - Fix LaTeX error with alphabetic delimiters in inline Listings (part of bug
   8985).