From 54e108f1efbc695a8da2f3ce33a44cff2eae4a36 Mon Sep 17 00:00:00 2001
From: Peter Varga <pvarga@inf.u-szeged.hu>
Date: Thu, 27 Apr 2017 11:29:46 +0200
Subject: [PATCH] Fix emitting selectionChanged signal for non-user text
selection change
This fixes the case when the text selection is triggered by JavaScript.
Text selection changes triggered by IME composition text replecement are
ignored. Test has been added for mouse selection clipboard: non-user
text selection should not update the clipboard.
Pulls in Chromium changes:
3deea95 Update TextSelection for non-user initiated events
Task-number: QTBUG-53134
Task-number: QTBUG-60381
Change-Id: Ib94f57a2aa61248fba75f595245fb388b9609b6c
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
src/3rdparty | 2 +-
src/core/render_widget_host_view_qt.cpp | 25 +++--
tests/auto/widgets/qwebengineview/BLACKLIST | 3 -
.../qwebengineview/tst_qwebengineview.cpp | 92 ++++++++++++-------
4 files changed, 78 insertions(+), 44 deletions(-)
diff --git a/src/3rdparty b/src/3rdparty
index dfaf15642..3deea958f 160000
--- a/src/3rdparty
+++ b/src/3rdparty
@@ -1 +1 @@
-Subproject commit dfaf1564283dcdd222be9717a1a5ad9389ec7f1b
+Subproject commit 3deea958f1485b50940e7437c0a3b73ed29ee134
diff --git a/src/core/render_widget_host_view_qt.cpp b/src/core/render_widget_host_view_qt.cpp
index 4e98a1016..f90a44044 100644
--- a/src/core/render_widget_host_view_qt.cpp
+++ b/src/core/render_widget_host_view_qt.cpp
@@ -757,8 +757,12 @@ void RenderWidgetHostViewQt::OnTextSelectionChanged(content::TextInputManager *t
Q_UNUSED(text_input_manager);
Q_UNUSED(updated_view);
+ const content::TextInputManager::TextSelection *selection = GetTextInputManager()->GetTextSelection(updated_view);
+ if (!selection)
+ return;
+
#if defined(USE_X11)
- if (!GetSelectedText().empty()) {
+ if (!GetSelectedText().empty() && selection->user_initiated()) {
// Set the CLIPBOARD_TYPE_SELECTION to the ui::Clipboard.
ui::ScopedClipboardWriter clipboard_writer(ui::CLIPBOARD_TYPE_SELECTION);
clipboard_writer.WriteText(GetSelectedText());
@@ -793,6 +797,18 @@ void RenderWidgetHostViewQt::selectionChanged()
return;
}
+ if (GetSelectedText().empty()) {
+ m_anchorPositionWithinSelection = m_cursorPosition;
+ m_cursorPositionWithinSelection = m_cursorPosition;
+
+ if (!m_emptyPreviousSelection) {
+ m_emptyPreviousSelection = GetSelectedText().empty();
+ m_adapterClient->selectionChanged();
+ }
+
+ return;
+ }
+
const content::TextInputManager::TextSelection *selection = text_input_manager_->GetTextSelection();
if (!selection)
return;
@@ -800,13 +816,6 @@ void RenderWidgetHostViewQt::selectionChanged()
if (!selection->range().IsValid())
return;
- // Avoid duplicate empty selectionChanged() signals
- if (GetSelectedText().empty() && m_emptyPreviousSelection) {
- m_anchorPositionWithinSelection = m_cursorPosition;
- m_cursorPositionWithinSelection = m_cursorPosition;
- return;
- }
-
int newAnchorPositionWithinSelection = 0;
int newCursorPositionWithinSelection = 0;
diff --git a/tests/auto/widgets/qwebengineview/BLACKLIST b/tests/auto/widgets/qwebengineview/BLACKLIST
index b3f393af4..a2ecd05b7 100644
--- a/tests/auto/widgets/qwebengineview/BLACKLIST
+++ b/tests/auto/widgets/qwebengineview/BLACKLIST
@@ -1,8 +1,5 @@
[doNotSendMouseKeyboardEventsWhenDisabled]
windows
-[imeComposition]
-osx
-
[inputFieldOverridesShortcuts]
osx
diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
index 8509e9a2d..0b49bd892 100644
--- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
+++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
@@ -33,6 +33,7 @@
#include <qdiriterator.h>
#include <qstackedlayout.h>
#include <qtemporarydir.h>
+#include <QClipboard>
#include <QCompleter>
#include <QLineEdit>
#include <QHBoxLayout>
@@ -145,6 +146,9 @@ private Q_SLOTS:
void imeCompositionQueryEvent_data();
void imeCompositionQueryEvent();
void newlineInTextarea();
+#ifndef QT_NO_CLIPBOARD
+ void globalMouseSelection();
+#endif
};
// This will be called before the first test function is executed.
@@ -1616,14 +1620,17 @@ void tst_QWebEngineView::textSelectionInInputField()
event.setCommitString("XXX", 0, 0);
QApplication::sendEvent(view.focusProxy(), &event);
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("QtWebEngineXXX"));
+ QCOMPARE(selectionChangedSpy.count(), 0);
event.setCommitString(QString(), -2, 2); // Erase two characters.
QApplication::sendEvent(view.focusProxy(), &event);
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("QtWebEngineX"));
+ QCOMPARE(selectionChangedSpy.count(), 0);
event.setCommitString(QString(), -1, 1); // Erase one character.
QApplication::sendEvent(view.focusProxy(), &event);
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("QtWebEngine"));
+ QCOMPARE(selectionChangedSpy.count(), 0);
// Move to the start of the line
QTest::keyClick(view.focusProxy(), Qt::Key_Home);
@@ -1793,12 +1800,7 @@ void tst_QWebEngineView::emptyInputMethodEvent()
QVERIFY(loadFinishedSpy.wait());
evaluateJavaScriptSync(view.page(), "var inputEle = document.getElementById('input1'); inputEle.focus(); inputEle.select();");
- QTRY_VERIFY(!evaluateJavaScriptSync(view.page(), "window.getSelection().toString()").toString().isEmpty());
-
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QVERIFY(selectionChangedSpy.wait(100));
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QCOMPARE(selectionChangedSpy.count(), 1);
+ QTRY_COMPARE(selectionChangedSpy.count(), 1);
// 1. Empty input method event does not clear text
QInputMethodEvent emptyEvent;
@@ -1845,12 +1847,7 @@ void tst_QWebEngineView::imeComposition()
QVERIFY(loadFinishedSpy.wait());
evaluateJavaScriptSync(view.page(), "var inputEle = document.getElementById('input1'); inputEle.focus(); inputEle.select();");
- QTRY_VERIFY(!evaluateJavaScriptSync(view.page(), "window.getSelection().toString()").toString().isEmpty());
-
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QVERIFY(selectionChangedSpy.wait(100));
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QCOMPARE(selectionChangedSpy.count(), 1);
+ QTRY_COMPARE(selectionChangedSpy.count(), 1);
// Clear the selection, also cancel the ongoing composition if there is one.
{
@@ -1859,17 +1856,13 @@ void tst_QWebEngineView::imeComposition()
attributes.append(newSelection);
QInputMethodEvent event("", attributes);
QApplication::sendEvent(view.focusProxy(), &event);
- QTRY_VERIFY(evaluateJavaScriptSync(view.page(), "window.getSelection().toString()").toString().isEmpty());
-
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QVERIFY(selectionChangedSpy.wait(100));
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
+ selectionChangedSpy.wait();
QCOMPARE(selectionChangedSpy.count(), 2);
}
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("QtWebEngine inputMethod"));
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImAnchorPosition).toInt(), 0);
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCursorPosition).toInt(), 0);
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCurrentSelection).toString(), QString(""));
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("QtWebEngine inputMethod"));
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImAnchorPosition).toInt(), 0);
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCursorPosition).toInt(), 0);
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCurrentSelection).toString(), QString(""));
selectionChangedSpy.clear();
@@ -1990,19 +1983,14 @@ void tst_QWebEngineView::imeComposition()
QList<QInputMethodEvent::Attribute> attributes;
QInputMethodEvent event("w", attributes);
QApplication::sendEvent(view.focusProxy(), &event);
- QTRY_VERIFY(evaluateJavaScriptSync(view.page(), "window.getSelection().toString()").toString().isEmpty());
- QTRY_COMPARE(evaluateJavaScriptSync(view.page(), "document.getElementById('input1').value").toString(), QString("oeQtWebEngine w"));
-
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QVERIFY(selectionChangedSpy.wait(100));
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
+ // The new composition should clear the previous selection
+ QVERIFY(selectionChangedSpy.wait());
QCOMPARE(selectionChangedSpy.count(), 2);
}
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("oeQtWebEngine "));
- QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCursorPosition).toInt(), 14);
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
- QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImAnchorPosition).toInt(), 14);
- QEXPECT_FAIL("", "https://bugreports.qt.io/browse/QTBUG-53134", Continue);
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImSurroundingText).toString(), QString("oeQtWebEngine "));
+ // The cursor should be positioned at the end of the composition text
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCursorPosition).toInt(), 15);
+ QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImAnchorPosition).toInt(), 15);
QCOMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCurrentSelection).toString(), QString(""));
// Send commit text, which makes the editor conforms composition.
@@ -2017,6 +2005,7 @@ void tst_QWebEngineView::imeComposition()
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCursorPosition).toInt(), 15);
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImAnchorPosition).toInt(), 15);
QTRY_COMPARE(view.focusProxy()->inputMethodQuery(Qt::ImCurrentSelection).toString(), QString(""));
+ QCOMPARE(selectionChangedSpy.count(), 2);
}
void tst_QWebEngineView::newlineInTextarea()
@@ -2196,5 +2185,44 @@ void tst_QWebEngineView::imeCompositionQueryEvent()
QTRY_COMPARE(anchorPosQuery.value(Qt::ImAnchorPosition).toInt(), 11);
}
+#ifndef QT_NO_CLIPBOARD
+void tst_QWebEngineView::globalMouseSelection()
+{
+ if (!QApplication::clipboard()->supportsSelection()) {
+ QSKIP("Test only relevant for systems with selection");
+ return;
+ }
+
+ QApplication::clipboard()->clear(QClipboard::Selection);
+ QWebEngineView view;
+ view.show();
+
+ QSignalSpy selectionChangedSpy(&view, SIGNAL(selectionChanged()));
+ QSignalSpy loadFinishedSpy(&view, SIGNAL(loadFinished(bool)));
+ view.setHtml("<html><body>"
+ " <input type='text' id='input1' value='QtWebEngine' size='50' />"
+ "</body></html>");
+ QVERIFY(loadFinishedSpy.wait());
+
+ // Select text via JavaScript
+ evaluateJavaScriptSync(view.page(), "var inputEle = document.getElementById('input1'); inputEle.focus(); inputEle.select();");
+ QTRY_COMPARE(selectionChangedSpy.count(), 1);
+ QVERIFY(QApplication::clipboard()->text(QClipboard::Selection).isEmpty());
+
+ // Deselect the selection (this moves the current cursor to the end of the text)
+ QPoint textInputCenter = elementCenter(view.page(), "input1");
+ QTest::mouseClick(view.focusProxy(), Qt::LeftButton, 0, textInputCenter);
+ QVERIFY(selectionChangedSpy.wait());
+ QCOMPARE(selectionChangedSpy.count(), 2);
+ QVERIFY(QApplication::clipboard()->text(QClipboard::Selection).isEmpty());
+
+ // Select to the start of the line
+ QTest::keyClick(view.focusProxy(), Qt::Key_Home, Qt::ShiftModifier);
+ QVERIFY(selectionChangedSpy.wait());
+ QCOMPARE(selectionChangedSpy.count(), 3);
+ QCOMPARE(QApplication::clipboard()->text(QClipboard::Selection), QStringLiteral("QtWebEngine"));
+}
+#endif
+
QTEST_MAIN(tst_QWebEngineView)
#include "tst_qwebengineview.moc"
--
GitLab