I don't like the reference to the BufferView cursor. cursorX gets a arbitrary CursorSlice. It shouldn't depend on the cursor or some other sideconditions.

If I understand correctly: cursorX is called for the cursor itself and for the selection anchor. For the latter it makes a difference if the cursor is inside the inset or outside. So somehow this fix looks like being at the wrong position, i.e. it should be handled where the cursorX for the selection is called. Otherwise in 6 months somebody else will fall over another symptom fix without a real cure of the problem.

If I am wrong, correct me. I don't know the code with the selections and cursor placement too well yet.

Stefan

2) I can't say about the actual method used for determining whether we're inside the cursor. Andre', JMarc --- does this seem OK to you? Is there a better way to achieve this?

Anyhow, I don't have commit access, so I can't commit. But the concept of this patch has certainly got my OK, and I'd like to see it go in.

I'm attaching a patch with a revised comment, based on what I said above, and on my understanding of the situation. Elazar, is it okay with you?

Dov
Index: src/Text.cpp
===================================================================
--- src/Text.cpp        (revision 18497)
+++ src/Text.cpp        (working copy)
@@ -1696,8 +1696,25 @@
        // edge (bidi!) -- MV
        if (sl.pos() < par.size()) {
                font = getFont(buffer, par, sl.pos());
+               // The cursor position inside an inset is expected to be
+               // relative to the inset's left edge. In RTL paragraphs,
+               // however, we currently have the position relative to the
+               // inset's *right* edge. We must explicitly correct the x
+               // position, by removing the width of the inset itself.
+               //
+               // Naturally, this correction should only take place *inside*
+               // the inset. To test whether we are actually inside the inset
+               // (and not only "at" the inset; i.e., at the position at
+               // which the inset is located in the surrounding paragraph,
+               // but not yet inside the inset), we compare the cursor slice
+               // stored at the cursor, to the cursor slice provided as a
+               // parameter. If they are equal, that means we're not in the
+               // inset which is at character sl.pos() in the pargraph, but
+               // we're right before it. Otherwise, it means that the inset
+               // was pushed onto the cursor's stack, and we must be inside.
                if (!boundary && font.isVisibleRightToLeft()
-                 && par.isInset(sl.pos()))
+                 && par.isInset(sl.pos())
+                 && (&bv.cursor().top())!=&sl)
                        x -= par.getInset(sl.pos())->width();
        }
        return int(x);

Attachment: PGP.sig
Description: Signierter Teil der Nachricht

Reply via email to