From 9d962bc9424c71a451d04741619c9597f3da8010 Mon Sep 17 00:00:00 2001
From: Alexandru Croitor <alexandru.croitor@qt.io>
Date: Tue, 9 May 2017 14:52:57 +0200
Subject: [PATCH] Fix selectedText() to return the value of what findText("")
finds
Previously when a find operation was finished by calling findText(""),
the selection buffer was cleared and calling selectedText() would
return an empty string, even though on the UI side a blue rectangle
selection was still visible.
This was due to an incorrect Unselect() call in
WebContentsAdapter::stopFinding().
With the new selection change support in the parent patch, and with the
removed Unselect() call, selectedText() now properly returns the value
of the highlighted blue selection after searching is finished.
Task-number: QTBUG-60673
Task-number: QTBUG-54071
Task-number: QTBUG-53134
Change-Id: I89e0eddb0c14af6d6c06ee878e706be65d3b0831
Reviewed-by: Viktor Engelmann <viktor.engelmann@qt.io>
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
---
src/core/web_contents_adapter.cpp | 3 -
.../qwebenginepage/tst_qwebenginepage.cpp | 62 +++++++++++++------
2 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 2b7ab03dd..7e93ffab9 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -916,9 +916,6 @@ void WebContentsAdapter::stopFinding()
{
Q_D(WebContentsAdapter);
d->webContentsDelegate->setLastSearchedString(QString());
- // Clear any previous selection,
- // but keep the renderer blue rectangle selection just like Chromium does.
- d->webContents->CollapseSelection();
d->webContents->StopFinding(content::STOP_FIND_ACTION_KEEP_SELECTION);
}
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index 837481fad..59cbc978b 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -2135,29 +2135,55 @@ void tst_QWebEnginePage::testStopScheduledPageRefresh()
void tst_QWebEnginePage::findText()
{
- QSignalSpy loadSpy(m_page, SIGNAL(loadFinished(bool)));
- m_page->setHtml(QString("<html><head></head><body><div>foo bar</div></body></html>"));
+ QSignalSpy loadSpy(m_view, SIGNAL(loadFinished(bool)));
+ m_view->setHtml(QString("<html><head></head><body><div>foo bar</div></body></html>"));
+
+ // Showing is required, otherwise all find operations fail.
+ m_view->show();
QTRY_COMPARE(loadSpy.count(), 1);
// Select whole page contents.
- m_page->triggerAction(QWebEnginePage::SelectAll);
- QTRY_COMPARE(m_page->hasSelection(), true);
+ m_view->page()->triggerAction(QWebEnginePage::SelectAll);
+ QTRY_COMPARE(m_view->hasSelection(), true);
- // Invoke a stopFinding() operation, which should clear the currently selected text.
- m_page->findText("");
- QTRY_VERIFY(m_page->selectedText().isEmpty());
+ // Invoking a stopFinding operation will not change or clear the currently selected text,
+ // if nothing was found beforehand.
+ {
+ CallbackSpy<bool> spy;
+ m_view->findText("", 0, spy.ref());
+ QVERIFY(spy.wasCalled());
+ QTRY_COMPARE(m_view->selectedText(), QString("foo bar"));
+ }
- QStringList words = (QStringList() << "foo" << "bar");
- foreach (QString subString, words) {
- // Invoke a find operation, which should clear the currently selected text, should
- // highlight all the found ocurrences, but should not update the selected text to the
- // searched for string.
- m_page->findText(subString);
- QTRY_VERIFY(m_page->selectedText().isEmpty());
-
- // Search highlights should be cleared, selected text should still be empty.
- m_page->findText("");
- QTRY_VERIFY(m_page->selectedText().isEmpty());
+ // Invoking a startFinding operation with text that won't be found, will clear the current
+ // selection.
+ {
+ CallbackSpy<bool> spy;
+ m_view->findText("Will not be found", 0, spy.ref());
+ QCOMPARE(spy.waitForResult(), false);
+ QTRY_VERIFY(m_view->selectedText().isEmpty());
+ }
+
+ // Select whole page contents again.
+ m_view->page()->triggerAction(QWebEnginePage::SelectAll);
+ QTRY_COMPARE(m_view->hasSelection(), true);
+
+ // Invoking a startFinding operation with text that will be found, will clear the current
+ // selection as well.
+ {
+ CallbackSpy<bool> spy;
+ m_view->findText("foo", 0, spy.ref());
+ QVERIFY(spy.waitForResult());
+ QTRY_VERIFY(m_view->selectedText().isEmpty());
+ }
+
+ // Invoking a stopFinding operation after text was found, will set the selected text to the
+ // found text.
+ {
+ CallbackSpy<bool> spy;
+ m_view->findText("", 0, spy.ref());
+ QTRY_VERIFY(spy.wasCalled());
+ QTRY_COMPARE(m_view->selectedText(), QString("foo"));
}
}
--
GitLab