From 4c305cba0bf7cc021fd355af574b431f0d5a057d Mon Sep 17 00:00:00 2001
From: Alexandru Croitor <alexandru.croitor@theqtcompany.com>
Date: Mon, 9 May 2016 09:26:27 +0200
Subject: [PATCH] Disabled WebEngine views should not receive focus.
Currently if a QWebEngineView or a QQuickWebEngineView is disabled
using setEnabled(false), after loading a web page, the views are
automatically focused, and a user might see a blinking caret in an html
input for example, even though the user can't interact with it.
Fix consists in not calling the Focus() method whenever a view is
disabled.
Change-Id: I1014fb5898a5ddf01a4e9b14c3eaf5d4006e5131
Task-number: QTBUG-53159
Reviewed-by: Michal Klocek <michal.klocek@theqtcompany.com>
---
src/core/web_contents_adapter_client.h | 1 +
src/core/web_contents_view_qt.cpp | 2 ++
src/webengine/api/qquickwebengineview.cpp | 6 ++++
src/webengine/api/qquickwebengineview_p_p.h | 1 +
src/webenginewidgets/api/qwebenginepage.cpp | 9 ++++++
src/webenginewidgets/api/qwebenginepage_p.h | 1 +
.../tst_qquickwebengineview.cpp | 30 +++++++++++++++++
.../qwebengineview/tst_qwebengineview.cpp | 32 +++++++++++++++++++
8 files changed, 82 insertions(+)
diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index f32e797a5..e6d25a8fb 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -239,6 +239,7 @@ public:
virtual void renderProcessTerminated(RenderProcessTerminationStatus terminationStatus, int exitCode) = 0;
virtual void requestGeometryChange(const QRect &geometry) = 0;
virtual void allowCertificateError(const QSharedPointer<CertificateErrorController> &errorController) = 0;
+ virtual bool isEnabled() const = 0;
virtual QSharedPointer<BrowserContextAdapter> browserContextAdapter() = 0;
diff --git a/src/core/web_contents_view_qt.cpp b/src/core/web_contents_view_qt.cpp
index 67addacd5..ae53619ac 100644
--- a/src/core/web_contents_view_qt.cpp
+++ b/src/core/web_contents_view_qt.cpp
@@ -113,6 +113,8 @@ void WebContentsViewQt::GetContainerBounds(gfx::Rect* out) const
void WebContentsViewQt::Focus()
{
+ if (!m_client->isEnabled())
+ return;
if (content::RenderWidgetHostView *rwhv = m_webContents->GetRenderWidgetHostView())
rwhv->Focus();
m_client->focusContainer();
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index a9ca47598..6396e1151 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -1001,6 +1001,12 @@ void QQuickWebEngineViewPrivate::renderProcessTerminated(
renderProcessExitStatus(terminationStatus)), exitCode);
}
+bool QQuickWebEngineViewPrivate::isEnabled() const
+{
+ const Q_Q(QQuickWebEngineView);
+ return q->isEnabled();
+}
+
bool QQuickWebEngineView::isLoading() const
{
Q_D(const QQuickWebEngineView);
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 65360dcac..831f70afd 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -171,6 +171,7 @@ public:
virtual void renderProcessTerminated(RenderProcessTerminationStatus terminationStatus,
int exitCode) Q_DECL_OVERRIDE;
virtual void requestGeometryChange(const QRect &geometry) Q_DECL_OVERRIDE { Q_UNUSED(geometry); }
+ virtual bool isEnabled() const Q_DECL_OVERRIDE;
virtual QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContextAdapter() Q_DECL_OVERRIDE;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 7795cc41d..2fd026682 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -1071,6 +1071,15 @@ void QWebEnginePagePrivate::requestGeometryChange(const QRect &geometry)
Q_EMIT q->geometryChangeRequested(geometry);
}
+bool QWebEnginePagePrivate::isEnabled() const
+{
+ const Q_Q(QWebEnginePage);
+ const QWidget *view = q->view();
+ if (view)
+ return view->isEnabled();
+ return true;
+}
+
QMenu *QWebEnginePage::createStandardContextMenu()
{
Q_D(QWebEnginePage);
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 3738de3cf..8e5be51d0 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -124,6 +124,7 @@ public:
virtual void renderProcessTerminated(RenderProcessTerminationStatus terminationStatus,
int exitCode) Q_DECL_OVERRIDE;
virtual void requestGeometryChange(const QRect &geometry) Q_DECL_OVERRIDE;
+ virtual bool isEnabled() const Q_DECL_OVERRIDE;
virtual QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContextAdapter() Q_DECL_OVERRIDE;
diff --git a/tests/auto/quick/qquickwebengineview/tst_qquickwebengineview.cpp b/tests/auto/quick/qquickwebengineview/tst_qquickwebengineview.cpp
index 60e495137..d39a6df54 100644
--- a/tests/auto/quick/qquickwebengineview/tst_qquickwebengineview.cpp
+++ b/tests/auto/quick/qquickwebengineview/tst_qquickwebengineview.cpp
@@ -61,6 +61,8 @@ private Q_SLOTS:
void inputMethodHints();
void basicRenderingSanity();
void setZoomFactor();
+ void stopSettingFocusWhenDisabled();
+ void stopSettingFocusWhenDisabled_data();
private:
inline QQuickWebEngineView *newWebEngineView();
@@ -481,5 +483,33 @@ void tst_QQuickWebEngineView::setZoomFactor()
QVERIFY(qFuzzyCompare(view->zoomFactor(), 2.5));
}
+void tst_QQuickWebEngineView::stopSettingFocusWhenDisabled()
+{
+ QFETCH(bool, viewEnabled);
+ QFETCH(bool, activeFocusResult);
+
+ QQuickWebEngineView *view = webEngineView();
+ m_window->show();
+ view->setSize(QSizeF(640, 480));
+ view->setEnabled(viewEnabled);
+ view->loadHtml("<html><head><title>Title</title></head><body>Hello"
+ "<input id=\"input\" type=\"text\"></body></html>");
+ QVERIFY(waitForLoadSucceeded(view));
+
+ // When enabled, the view should get active focus after the page is loaded.
+ QTRY_COMPARE_WITH_TIMEOUT(view->hasActiveFocus(), activeFocusResult, 1000);
+ view->runJavaScript("document.getElementById(\"input\").focus()");
+ QTRY_COMPARE_WITH_TIMEOUT(view->hasActiveFocus(), activeFocusResult, 1000);
+}
+
+void tst_QQuickWebEngineView::stopSettingFocusWhenDisabled_data()
+{
+ QTest::addColumn<bool>("viewEnabled");
+ QTest::addColumn<bool>("activeFocusResult");
+
+ QTest::newRow("enabled view gets active focus") << true << true;
+ QTest::newRow("disabled view does not get active focus") << false << false;
+}
+
QTEST_MAIN(tst_QQuickWebEngineView)
#include "tst_qquickwebengineview.moc"
diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
index d496362f9..e66bf18cd 100644
--- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
+++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
@@ -70,6 +70,8 @@ private Q_SLOTS:
#endif
void doNotSendMouseKeyboardEventsWhenDisabled();
void doNotSendMouseKeyboardEventsWhenDisabled_data();
+ void stopSettingFocusWhenDisabled();
+ void stopSettingFocusWhenDisabled_data();
};
// This will be called before the first test function is executed.
@@ -709,5 +711,35 @@ void tst_QWebEngineView::doNotSendMouseKeyboardEventsWhenDisabled_data()
QTest::newRow("disabled view does not receive events") << false << 4;
}
+void tst_QWebEngineView::stopSettingFocusWhenDisabled()
+{
+ QFETCH(bool, viewEnabled);
+ QFETCH(bool, focusResult);
+
+ QWebEngineView webView;
+ webView.resize(640, 480);
+ webView.show();
+ webView.setEnabled(viewEnabled);
+ QTest::qWaitForWindowExposed(&webView);
+
+ QSignalSpy loadSpy(&webView, SIGNAL(loadFinished(bool)));
+ webView.setHtml("<html><head><title>Title</title></head><body>Hello"
+ "<input id=\"input\" type=\"text\"></body></html>");
+ QTRY_COMPARE(loadSpy.count(), 1);
+
+ QTRY_COMPARE_WITH_TIMEOUT(webView.hasFocus(), focusResult, 1000);
+ evaluateJavaScriptSync(webView.page(), "document.getElementById(\"input\").focus()");
+ QTRY_COMPARE_WITH_TIMEOUT(webView.hasFocus(), focusResult, 1000);
+}
+
+void tst_QWebEngineView::stopSettingFocusWhenDisabled_data()
+{
+ QTest::addColumn<bool>("viewEnabled");
+ QTest::addColumn<bool>("focusResult");
+
+ QTest::newRow("enabled view gets focus") << true << true;
+ QTest::newRow("disabled view does not get focus") << false << false;
+}
+
QTEST_MAIN(tst_QWebEngineView)
#include "tst_qwebengineview.moc"
--
GitLab