commit d4faf127d74643ce10a1a5240ea02601336e48fe
Author: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date:   Tue May 27 15:14:14 2014 +0200

    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 a0f8f72..7f82675 100644
--- 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 66d310f..2acded9 100644
--- 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 3859a2c..da94c47 100644
--- 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).
 

Reply via email to