[network/ruqola] src/widgets: Fix selection text when we have text in attachment url

Git commit 4210441d6589e17fc3278c6836075a3fe9f1d0c6 by Laurent Montel.
Committed on 26/07/2026 at 21:33.
Pushed by mlaurent into branch 'master'.

Fix selection text when we have text in attachment url

M  +58   -8    src/widgets/delegateutils/textselection.cpp
M  +122  -0    src/widgets/room/autotests/textselectiontest.cpp
M  +4    -0    src/widgets/room/autotests/textselectiontest.h

https://invent.kde.org/network/ruqola/-/commit/4210441d6589e17fc3278c6836075a3fe9f1d0c6

diff --git a/src/widgets/delegateutils/textselection.cpp b/src/widgets/delegateutils/textselection.cpp
index 8131e0a14a..bd3a45c1d8 100644
--- a/src/widgets/delegateutils/textselection.cpp
+++ b/src/widgets/delegateutils/textselection.cpp
@@ -5,7 +5,6 @@
 */
 
 #include "textselection.h"
-using namespace Qt::Literals::StringLiterals;
 
 #include "messages/message.h"
 #include "model/messagesmodel.h"
@@ -15,6 +14,7 @@ using namespace Qt::Literals::StringLiterals;
 #include <QTextDocument>
 #include <QTextDocumentFragment>
 
+using namespace Qt::Literals::StringLiterals;
 TextSelection::TextSelection() = default;
 
 DocumentFactoryInterface::~DocumentFactoryInterface() = default;
@@ -159,10 +159,24 @@ QTextCursor TextSelection::selectionForIndex(const QModelIndex &index, QTextDocu
     if (!hasSelection()) {
         return {};
     }
+    if (!doc) {
+        return {};
+    }
     Q_ASSERT(index.model() == mStartIndex.model());
     Q_ASSERT(index.model() == mEndIndex.model());
 
-    if (att.isValid() && mAttachmentSelection.isEmpty() && mMessageUrlSelection.isEmpty() && !msgUrl.hasHtmlDescription()) {
+    const bool selectionStartedOutsideText = mStartPos < 0;
+
+    if (att.isValid()) {
+        if (mAttachmentSelection.isEmpty()) {
+            return {};
+        }
+    } else if (msgUrl.hasHtmlDescription()) {
+        if (mMessageUrlSelection.isEmpty()) {
+            return {};
+        }
+    } else if (mEndPos < 0) {
+        // Selection endpoint is still in attachment/url preview, so main message text is not selected.
         return {};
     }
     const OrderedPositions ordered = orderedPositions();
@@ -170,38 +184,60 @@ QTextCursor TextSelection::selectionForIndex(const QModelIndex &index, QTextDocu
     int toCharPos = ordered.toCharPos;
     // qDebug() << "BEFORE toCharPos" << toCharPos << " fromCharPos " << fromCharPos;
     QTextCursor cursor(doc);
+    const int maxCharPos = qMax(0, doc->characterCount() - 1);
+
+    if (selectionStartedOutsideText && !att.isValid() && !msgUrl.hasHtmlDescription() && fromCharPos < 0) {
+        // Selection started below/above text (attachment or URL preview): entering text from outside
+        // should anchor from the closest edge of the text document (the end for reverse drag-up).
+        fromCharPos = maxCharPos;
+    }
+    if (selectionStartedOutsideText && !att.isValid() && !msgUrl.hasHtmlDescription() && toCharPos < 0) {
+        // If the opposite text endpoint is invalid (still represented by the original URL/attachment start),
+        // keep the full text side selected instead of collapsing to position 0.
+        toCharPos = maxCharPos;
+    }
 
     if (att.isValid()) {
+        bool foundAttachmentSelection = false;
         for (const AttachmentSelection &attSelection : std::as_const(mAttachmentSelection)) {
             if (attSelection.attachment == att) {
                 fromCharPos = attSelection.fromCharPos;
                 toCharPos = attSelection.toCharPos;
+                foundAttachmentSelection = true;
                 // qDebug() << "ATTACHMENT toCharPos" << toCharPos << " fromCharPos " << fromCharPos;
                 break;
             }
         }
+        if (!foundAttachmentSelection) {
+            return {};
+        }
     }
     if (msgUrl.hasHtmlDescription()) {
+        bool foundMessageUrlSelection = false;
         for (const MessageUrlSelection &messageUrlSelection : std::as_const(mMessageUrlSelection)) {
             if (messageUrlSelection.messageUrl == msgUrl) {
                 fromCharPos = messageUrlSelection.fromCharPos;
                 toCharPos = messageUrlSelection.toCharPos;
+                foundMessageUrlSelection = true;
                 // qDebug() << "MessageUrl toCharPos" << toCharPos << " fromCharPos " << fromCharPos;
                 break;
             }
         }
+        if (!foundMessageUrlSelection) {
+            return {};
+        }
     }
 
     // qDebug() << "AFTER toCharPos" << toCharPos << " fromCharPos " << fromCharPos;
     const int row = index.row();
     if (row == ordered.fromRow)
-        cursor.setPosition(qMax(fromCharPos, 0));
+        cursor.setPosition(qBound(0, fromCharPos, maxCharPos));
     else if (row > ordered.fromRow)
         cursor.setPosition(0);
     else
         return {};
     if (row == ordered.toRow)
-        cursor.setPosition(qMax(toCharPos, 0), QTextCursor::KeepAnchor);
+        cursor.setPosition(qBound(0, toCharPos, maxCharPos), QTextCursor::KeepAnchor);
     else if (row < ordered.toRow)
         cursor.movePosition(QTextCursor::End, QTextCursor::KeepAnchor);
     else
@@ -247,6 +283,7 @@ void TextSelection::setAttachmentTextSelectionStart(const QModelIndex &index, in
     if (msgAttach.isValid()) {
         AttachmentSelection selection;
         selection.fromCharPos = charPos;
+        selection.toCharPos = charPos;
         selection.attachment = msgAttach;
         mAttachmentSelection.append(std::move(selection));
         // qDebug() << " start selection is in attachment ";
@@ -260,6 +297,7 @@ void TextSelection::setPreviewUrlTextSelectionStart(const QModelIndex &index, in
     if (msgUrl.hasHtmlDescription()) {
         MessageUrlSelection selection;
         selection.fromCharPos = charPos;
+        selection.toCharPos = charPos;
         selection.messageUrl = msgUrl;
         mMessageUrlSelection.append(std::move(selection));
         mStartPos = -1;
@@ -293,7 +331,15 @@ void TextSelection::setTextSelectionEnd(const QModelIndex &index, int charPos)
 
 void TextSelection::setAttachmentTextSelectionEnd(const QModelIndex &index, int charPos, const MessageAttachment &msgAttach)
 {
+    const bool keepTextSelectionEndPos = (mStartPos >= 0) && (mEndPos >= 0);
+    const int previousEndPos = mEndPos;
     setTextSelectionEnd(index, charPos);
+    if (keepTextSelectionEndPos) {
+        mEndPos = previousEndPos;
+    } else if (mStartPos < 0) {
+        // The drag endpoint is currently in attachment text; main text must stay unselected.
+        mEndPos = -1;
+    }
     if (msgAttach.isValid()) {
         const auto countAtt{mAttachmentSelection.count()};
         for (int i = 0; i < countAtt; ++i) {
@@ -301,7 +347,6 @@ void TextSelection::setAttachmentTextSelectionEnd(const QModelIndex &index, int
                 AttachmentSelection attachmentSelectFound = mAttachmentSelection.takeAt(i);
                 attachmentSelectFound.toCharPos = charPos;
                 mAttachmentSelection.append(std::move(attachmentSelectFound));
-                mEndPos = -1;
                 return;
             }
         }
@@ -310,13 +355,20 @@ void TextSelection::setAttachmentTextSelectionEnd(const QModelIndex &index, int
         selection.toCharPos = charPos;
         selection.attachment = msgAttach;
         mAttachmentSelection.append(std::move(selection));
-        mEndPos = -1;
     }
 }
 
 void TextSelection::setPreviewUrlTextSelectionEnd(const QModelIndex &index, int charPos, const MessageUrl &msgUrl)
 {
+    const bool keepTextSelectionEndPos = (mStartPos >= 0) && (mEndPos >= 0);
+    const int previousEndPos = mEndPos;
     setTextSelectionEnd(index, charPos);
+    if (keepTextSelectionEndPos) {
+        mEndPos = previousEndPos;
+    } else if (mStartPos < 0) {
+        // The drag endpoint is currently in URL preview text; main text must stay unselected.
+        mEndPos = -1;
+    }
     if (msgUrl.hasHtmlDescription()) {
         const auto countMessageUrl{mMessageUrlSelection.count()};
         for (int i = 0; i < countMessageUrl; ++i) {
@@ -324,7 +376,6 @@ void TextSelection::setPreviewUrlTextSelectionEnd(const QModelIndex &index, int
                 MessageUrlSelection messageUrlSelectFound = mMessageUrlSelection.takeAt(i);
                 messageUrlSelectFound.toCharPos = charPos;
                 mMessageUrlSelection.append(std::move(messageUrlSelectFound));
-                mEndPos = -1;
                 return;
             }
         }
@@ -333,7 +384,6 @@ void TextSelection::setPreviewUrlTextSelectionEnd(const QModelIndex &index, int
         selection.toCharPos = charPos;
         selection.messageUrl = msgUrl;
         mMessageUrlSelection.append(std::move(selection));
-        mEndPos = -1;
     }
 }
 
diff --git a/src/widgets/room/autotests/textselectiontest.cpp b/src/widgets/room/autotests/textselectiontest.cpp
index 98533b9bd0..e9eb021467 100644
--- a/src/widgets/room/autotests/textselectiontest.cpp
+++ b/src/widgets/room/autotests/textselectiontest.cpp
@@ -7,6 +7,7 @@
 #include "textselectiontest.h"
 
 #include "delegateutils/textselection.h"
+#include "messages/messageurl.h"
 #include "model/messagesmodel.h"
 
 #include <QSignalSpy>
@@ -222,6 +223,127 @@ void TextSelectionTest::testSelectAll()
     QCOMPARE(selection.selectedText(TextSelection::Format::Text), u"Line 1 bold"_s);
 }
 
+void TextSelectionTest::testSelectionForIndexDoesNotIncludeUrlPreviewByDefault()
+{
+    const QModelIndex index1 = model.index(1, 0);
+    TestFactory factory(model.rowCount());
+    TextSelection selection;
+    selection.setTextHelperFactory(&factory);
+
+    selection.setTextSelectionStart(index1, 0);
+    selection.setTextSelectionEnd(index1, 4);
+
+    MessageUrl messageUrl;
+    messageUrl.setUrl(u"https://kde.org"_s);
+    messageUrl.setPageTitle(u"KDE"_s);
+    messageUrl.setDescription(u"Community"_s);
+    messageUrl.generateMessageUrlInfo();
+    QVERIFY(messageUrl.hasHtmlDescription());
+
+    QTextDocument urlPreviewDoc;
+    urlPreviewDoc.setHtml(messageUrl.htmlDescription());
+
+    const QTextCursor cursor = selection.selectionForIndex(index1, &urlPreviewDoc, {}, messageUrl);
+    QVERIFY(cursor.isNull());
+}
+
+void TextSelectionTest::testSelectionExtendingToUrlPreviewKeepsTextSelection()
+{
+    const QModelIndex index1 = model.index(1, 0);
+    TestFactory factory(model.rowCount());
+    TextSelection selection;
+    selection.setTextHelperFactory(&factory);
+
+    selection.setTextSelectionStart(index1, 0);
+    selection.setTextSelectionEnd(index1, 4);
+
+    MessageUrl messageUrl;
+    messageUrl.setUrl(u"https://kde.org"_s);
+    messageUrl.setPageTitle(u"KDE"_s);
+    messageUrl.setDescription(u"Community"_s);
+    messageUrl.generateMessageUrlInfo();
+    QVERIFY(messageUrl.hasHtmlDescription());
+
+    selection.setPreviewUrlTextSelectionEnd(index1, 366, messageUrl);
+
+    QTextDocument urlPreviewDoc;
+    urlPreviewDoc.setHtml(messageUrl.htmlDescription());
+
+    const QTextCursor messageCursor = selection.selectionForIndex(index1, factory.documentForIndex(index1));
+    QVERIFY(!messageCursor.isNull());
+    QCOMPARE(messageCursor.selection().toPlainText(), u"Line"_s);
+
+    const QTextCursor urlCursor = selection.selectionForIndex(index1, &urlPreviewDoc, {}, messageUrl);
+    QVERIFY(!urlCursor.isNull());
+}
+
+void TextSelectionTest::testSelectionStartingInUrlPreviewAndMovingToText()
+{
+    const QModelIndex index1 = model.index(1, 0);
+    TestFactory factory(model.rowCount());
+    TextSelection selection;
+    selection.setTextHelperFactory(&factory);
+
+    MessageUrl messageUrl;
+    messageUrl.setUrl(u"https://kde.org"_s);
+    messageUrl.setPageTitle(u"KDE"_s);
+    messageUrl.setDescription(u"Community"_s);
+    messageUrl.generateMessageUrlInfo();
+    QVERIFY(messageUrl.hasHtmlDescription());
+
+    QTextDocument urlPreviewDoc;
+    urlPreviewDoc.setHtml(messageUrl.htmlDescription());
+
+    selection.setPreviewUrlTextSelectionStart(index1, 366, messageUrl);
+
+    // Move the selection endpoint to the main message text (drag up).
+    selection.setTextSelectionEnd(index1, 4);
+
+    const QTextCursor messageCursor = selection.selectionForIndex(index1, factory.documentForIndex(index1));
+    QVERIFY(!messageCursor.isNull());
+    QCOMPARE(messageCursor.selection().toPlainText(), u" 1 bold"_s);
+
+    const QTextCursor urlCursor = selection.selectionForIndex(index1, &urlPreviewDoc, {}, messageUrl);
+    QVERIFY(!urlCursor.isNull());
+    // Starting in URL preview and immediately dragging to message text can keep a zero-length URL cursor.
+    QCOMPARE(urlCursor.position(), urlCursor.anchor());
+}
+
+void TextSelectionTest::testSelectionStartingInUrlPreviewAndMovingToPreviousMessage()
+{
+    const QModelIndex index0 = model.index(0, 0);
+    const QModelIndex index1 = model.index(1, 0);
+    TestFactory factory(model.rowCount());
+    TextSelection selection;
+    selection.setTextHelperFactory(&factory);
+
+    MessageUrl messageUrl;
+    messageUrl.setUrl(u"https://kde.org"_s);
+    messageUrl.setPageTitle(u"KDE"_s);
+    messageUrl.setDescription(u"Community"_s);
+    messageUrl.generateMessageUrlInfo();
+    QVERIFY(messageUrl.hasHtmlDescription());
+
+    selection.setPreviewUrlTextSelectionStart(index1, 366, messageUrl);
+
+    // Move the selection endpoint to the previous message row.
+    selection.setTextSelectionEnd(index0, 4);
+
+    const QTextCursor row0Cursor = selection.selectionForIndex(index0, factory.documentForIndex(index0));
+    QVERIFY(!row0Cursor.isNull());
+    QCOMPARE(row0Cursor.selection().toPlainText(), u" 0"_s);
+
+    const QTextCursor row1Cursor = selection.selectionForIndex(index1, factory.documentForIndex(index1));
+    QVERIFY(!row1Cursor.isNull());
+    QCOMPARE(row1Cursor.selection().toPlainText(), u"Line 1 bold"_s);
+
+    QTextDocument urlPreviewDoc;
+    urlPreviewDoc.setHtml(messageUrl.htmlDescription());
+    const QTextCursor urlCursor = selection.selectionForIndex(index1, &urlPreviewDoc, {}, messageUrl);
+    QVERIFY(!urlCursor.isNull());
+    QVERIFY(!urlCursor.selection().toPlainText().isEmpty());
+}
+
 void TextSelectionTest::textClear()
 {
     // GIVEN
diff --git a/src/widgets/room/autotests/textselectiontest.h b/src/widgets/room/autotests/textselectiontest.h
index 347dece16d..daea08e110 100644
--- a/src/widgets/room/autotests/textselectiontest.h
+++ b/src/widgets/room/autotests/textselectiontest.h
@@ -23,6 +23,10 @@ private Q_SLOTS:
     void testSelectWordUnderCursor();
     void shouldHaveDefaultValues();
     void testSelectAll();
+    void testSelectionForIndexDoesNotIncludeUrlPreviewByDefault();
+    void testSelectionExtendingToUrlPreviewKeepsTextSelection();
+    void testSelectionStartingInUrlPreviewAndMovingToText();
+    void testSelectionStartingInUrlPreviewAndMovingToPreviousMessage();
     void textClear();
 
 private: