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

diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index a0f8f72c0e97f5cc5361e54d384d09ed9461a968..7f826756cba6010dfcd7415b3e1082ed72f870ae 100644 (file)
--- a/src/TextMetrics.cpp
+++ b/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;
-
-       // 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);
-       }
+       Inset * inset = it->inset;
+       //lyxerr << "inset " << inset << " hit at x: " << x << " y: " << y << endl;
+
+       // 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
 {

diff --git a/src/TextMetrics.h b/src/TextMetrics.h
index 66d310fcc60ef4164c23152341d88b2385da8f3f..2acded9067be39194ad9062ac6e7ad78dddf78a8 100644 (file)
--- a/src/TextMetrics.h
+++ b/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.

diff --git a/status.21x b/status.21x
index 3859a2c3d32177b48f59e6ba6665784f306cb0f1..da94c4783ebb8b1f8dfb4b34356887d5a38f6a54 100644 (file)
--- a/status.21x
+++ b/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).