From 0fa3aba5c6ebf40738abfd3fa480c046f787766b Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Thu, 9 Jan 2025 12:00:04 +0100
Subject: [PATCH] Check that paragraph has a position when computing cursor
 position

Rename BufferView::paragraphVisible to BufferView::hasPosition to
reflect better its semantics. Add a check on the validity of paragraph
position in metrics.

In Cursor::saveBeforeDispatchPosXY, check BufferView::hasPosition
before computing position.

Finally, use caretInView in GuiCompleter::updatePopup, since this is
what is really needed here.

There still remains cases where getPos() is called without a proper
check of the metrics beforehand. Time will tell whether they are a problem.
---
 src/BufferView.cpp                | 8 ++++----
 src/BufferView.h                  | 4 ++--
 src/Cursor.cpp                    | 6 +++++-
 src/frontends/qt/GuiCompleter.cpp | 3 +--
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index e7aa2826c9..2def0484fd 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -3455,7 +3455,7 @@ Point BufferView::coordOffset(DocIterator const & dit) const
 
 Point BufferView::getPos(DocIterator const & dit) const
 {
-	if (!paragraphVisible(dit))
+	if (!hasPosition(dit))
 		return Point(-1, -1);
 
 	CursorSlice const & bot = dit.bottom();
@@ -3468,12 +3468,12 @@ Point BufferView::getPos(DocIterator const & dit) const
 }
 
 
-bool BufferView::paragraphVisible(DocIterator const & dit) const
+bool BufferView::hasPosition(DocIterator const & dit) const
 {
 	CursorSlice const & bot = dit.bottom();
 	TextMetrics const & tm = textMetrics(bot.text());
 
-	return tm.contains(bot.pit());
+	return tm.contains(bot.pit()) && tm.parMetrics(bot.pit()).hasPosition();
 }
 
 
@@ -3585,7 +3585,7 @@ frontend::CaretGeometry const &  BufferView::caretGeometry() const
 
 bool BufferView::caretInView() const
 {
-	if (!paragraphVisible(cursor()))
+	if (!hasPosition(cursor()))
 		return false;
 	Point p;
 	Dimension dim;
diff --git a/src/BufferView.h b/src/BufferView.h
index deeeb7061b..9099278128 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -339,8 +339,8 @@ public:
 
 	///
 	Point getPos(DocIterator const & dit) const;
-	/// is the paragraph of the cursor visible ?
-	bool paragraphVisible(DocIterator const & dit) const;
+	/// is there enough metrics information to compute iterator position?
+	bool hasPosition(DocIterator const & dit) const;
 	/// is the caret currently visible in the view
 	bool caretInView() const;
 	/// get the position and height of the caret
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index fa844b2946..be49141c99 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -772,7 +772,11 @@ bool Cursor::getStatus(FuncRequest const & cmd, FuncStatus & status) const
 
 void Cursor::saveBeforeDispatchPosXY()
 {
-	getPos(beforeDispatchPosX_, beforeDispatchPosY_);
+	// FIXME: it is known that the case where the position is not
+	// known exist, in particular with previews. The question is
+	// whether not saving the cursor position is bad in these cases.
+	if (bv().hasPosition(*this))
+		getPos(beforeDispatchPosX_, beforeDispatchPosY_);
 }
 
 
diff --git a/src/frontends/qt/GuiCompleter.cpp b/src/frontends/qt/GuiCompleter.cpp
index 01ae3ccec8..3fe4946336 100644
--- a/src/frontends/qt/GuiCompleter.cpp
+++ b/src/frontends/qt/GuiCompleter.cpp
@@ -431,8 +431,7 @@ void GuiCompleter::updatePopup(Cursor const & cur)
 void GuiCompleter::asyncUpdatePopup()
 {
 	Cursor cur = gui_->bufferView().cursor();
-	if (!cur.inset().completionSupported(cur)
-		  || !cur.bv().paragraphVisible(cur)) {
+	if (!cur.inset().completionSupported(cur) || !cur.bv().caretInView()) {
 		popupVisible_ = false;
 		return;
 	}