TerminalDisplay: Extract copying related functions

src/CMakeLists.txt

@@ -184,7 +184,7 @@ set(konsoleprivate_SRCS ${windowadaptors_SRCS}
                         terminalDisplay/TerminalScrollBar.cpp
                         terminalDisplay/TerminalColor.cpp
                         terminalDisplay/TerminalFonts.cpp
-                        terminalDisplay/TerminalPasting.cpp
+                        terminalDisplay/TerminalClipboard.cpp
 
                         widgets/TerminalDisplayAccessible.cpp
                         widgets/TerminalHeaderBar.cpp

src/terminalDisplay/TerminalPasting.cpp → src/terminalDisplay/TerminalClipboard.cpp

@@ -17,10 +17,9 @@
 #include <KShell>
 #include <KLocalizedString>
 
-#include "TerminalPasting.h"
+#include "TerminalClipboard.h"
 
-namespace Konsole {
-namespace terminalPasting {
+namespace Konsole::terminalClipboard {
 
 // Most code in Konsole uses UTF-32. We're filtering
 // UTF-16 here, as all control characters can be represented
@@ -142,5 +141,26 @@ bool isUnsafe(const QChar c) {
     return (c.category() == QChar::Category::Other_Control && std::find(ALLOWLIST.begin(), ALLOWLIST.end(), c.unicode()) != ALLOWLIST.end());
 }
 
+void copyToX11Selection(const QString &textToCopy, bool isHtml, bool autoCopySelectedText)
+{
+    if (textToCopy.isEmpty()) {
+        return;
+    }
+
+    auto mimeData = new QMimeData;
+    mimeData->setText(textToCopy);
+
+    if (isHtml) {
+        mimeData->setHtml(textToCopy);
+    }
+
+    if (QApplication::clipboard()->supportsSelection()) {
+        QApplication::clipboard()->setMimeData(mimeData, QClipboard::Selection);
+    }
+
+    if (autoCopySelectedText) {
+        QApplication::clipboard()->setMimeData(mimeData, QClipboard::Clipboard);
+    }
 }
+
 }

src/terminalDisplay/TerminalPasting.h → src/terminalDisplay/TerminalClipboard.h

@@ -15,8 +15,7 @@ class QStringList;
 
 #include <optional>
 
-namespace Konsole {
-namespace terminalPasting {
+namespace Konsole::terminalClipboard {
 
 /**
  * Retrieves the content of the clipboard and preprocesses it for pasting
@@ -48,5 +47,6 @@ QStringList checkForUnsafeCharacters(const QString &text);
  */
 bool isUnsafe(const QChar c);
 
-}
+void copyToX11Selection(const QString &textToCopy, bool copyAsHtml, bool autoCopySelectedText);
+
 }

src/terminalDisplay/TerminalDisplay.cpp

@@ -75,7 +75,7 @@
 #include "TerminalScrollBar.h"
 #include "TerminalColor.h"
 #include "TerminalFonts.h"
-#include "TerminalPasting.h"
+#include "TerminalClipboard.h"
 
 using namespace Konsole;
 
@@ -1565,7 +1565,7 @@ void TerminalDisplay::processMidButtonClick(QMouseEvent* ev)
         if (_middleClickPasteMode == Enum::PasteFromX11Selection) {
             pasteFromX11Selection(appendEnter);
         } else if (_middleClickPasteMode == Enum::PasteFromClipboard) {
-            doPaste(terminalPasting::pasteFromClipboard(), appendEnter);
+            doPaste(terminalClipboard::pasteFromClipboard(), appendEnter);
         } else {
             Q_ASSERT(false);
         }
@@ -2117,7 +2117,7 @@ void TerminalDisplay::doPaste(QString text, bool appendReturn)
         }
     }
 
-    auto unsafeCharacters = terminalPasting::checkForUnsafeCharacters(text);
+    auto unsafeCharacters = terminalClipboard::checkForUnsafeCharacters(text);
 
     if (!unsafeCharacters.isEmpty()) {
         int result = KMessageBox::warningYesNoCancelList(window(),
@@ -2140,7 +2140,7 @@ void TerminalDisplay::doPaste(QString text, bool appendReturn)
         case KMessageBox::Cancel:
             return;
         case KMessageBox::Yes: {
-            text = terminalPasting::sanitizeString(text);
+            text = terminalClipboard::sanitizeString(text);
         }
         case KMessageBox::No:
             break;
@@ -2149,7 +2149,7 @@ void TerminalDisplay::doPaste(QString text, bool appendReturn)
         }
     }
 
-    auto pasteString = terminalPasting::prepareStringForPasting(text, appendReturn, bracketedPasteMode());
+    auto pasteString = terminalClipboard::prepareStringForPasting(text, appendReturn, bracketedPasteMode());
     if (pasteString.has_value()) {
         // perform paste by simulating keypress events
         QKeyEvent e(QEvent::KeyPress, 0, Qt::NoModifier, text);
@@ -2178,26 +2178,11 @@ void TerminalDisplay::copyToX11Selection()
         return;
     }
 
+    const auto &text = _copyTextAsHTML ?
+                _screenWindow->selectedText(currentDecodingOptions() | Screen::ConvertToHtml)
+              : _screenWindow->selectedText(currentDecodingOptions());
 
-    const QString &text = _screenWindow->selectedText(currentDecodingOptions());
-    if (text.isEmpty()) {
-        return;
-    }
-
-    auto mimeData = new QMimeData;
-    mimeData->setText(text);
-
-    if (_copyTextAsHTML) {
-        mimeData->setHtml(_screenWindow->selectedText(currentDecodingOptions() | Screen::ConvertToHtml));
-    }
-
-    if (QApplication::clipboard()->supportsSelection()) {
-        QApplication::clipboard()->setMimeData(mimeData, QClipboard::Selection);
-    }
-
-    if (_autoCopySelectedText) {
-        QApplication::clipboard()->setMimeData(mimeData, QClipboard::Clipboard);
-    }
+    terminalClipboard::copyToX11Selection(text, _copyTextAsHTML, _autoCopySelectedText);
 }
 
 void TerminalDisplay::copyToClipboard()
@@ -2701,7 +2686,7 @@ void TerminalDisplay::setSessionController(SessionController* controller)
 {
     _sessionController = controller;
     connect(_sessionController, &Konsole::SessionController::pasteFromClipboardRequested, [this] {
-        doPaste(terminalPasting::pasteFromClipboard(), false);
+        doPaste(terminalClipboard::pasteFromClipboard(), false);
     });
     _headerBar->finishHeaderSetup(controller);
 }

[Laurent Montel]

Hi, I am not sure that it's this commit which breaks my konsole but I rebuild it this morning and now each time that I select to copy a text in konsole, and I use mouse middle button for pasting it paste text as "editorsendcheckplugins
" => it's not good

but CTRL+C/CTRL+V works fine.

Regardd

[David Faure]

I think the bug is clear: text (line 2181) is created with the ConvertToHtml flag, and used for both plain text and html. Before this commit, selectedText() was called twice, with different options, for plain text and for html. The copyToX11Selection method needs to take two QStrings as input, one for each format. I would do it, but I'm on the stable branch, not master :-)

@tcanabrava @jbbgameich

[Jonah Brüchert]

Thanks for investigating, I'll look into it.

PS: Sorry for breaking it in the first place.

[Jonah Brüchert]

@mlaurent I'm not 100% sure I understand the bug correctly, but can you test whether !421 (merged) fixes it for you?

[Ahmad Samir]

IIUC, the bug happens when you select text, then go to paste it with middle click, then you get the extra html tags. But if you copy the text, the paste it with e.g. Ctrl+V then everything works as expected and the copy text as HTML config option is respected.

[Laurent Montel]

@jbbgameich I confirm that !421 (merged) fix my bug. Thanks