From 58467ed1950ee070d0907cbdabb8466aba277305 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Tue, 13 Sep 2016 18:17:43 +0200 Subject: Fix select tag interaction when the web view is inside a modal dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously when a QWebEngineView was inside a modal QDialog, trying to click on a select tag option did not properly select the option. It either focused the new option without closing the popup, or didn't focus it at all. Fix consists in making sure the newly created popup QWindow and RenderWidgetHostViewQtDelegateWidget are marked as children of the QWebEngineView, so that they are considered part of the current modal session by the OS, thus allowing user interaction with them. Because the ownership of the delegate widget should still be retained by its respective RenderWidgetHostViewQt instance, the QObject parent of the delegate is unset before the parent is destroyed. Also to make it work on macOS, the window attribute has to be set to Qt::Tool instead of Qt::ToolTip. Change-Id: I56d6f446254a624428a0c661ac3c49eb409c931e Task-number: QTBUG-54836 Reviewed-by: Qt CI Bot Reviewed-by: Michael Brüning --- src/webenginewidgets/api/qwebenginepage.cpp | 11 ++++++++++- .../render_widget_host_view_qt_delegate_widget.cpp | 21 ++++++++++++++++++++- .../render_widget_host_view_qt_delegate_widget.h | 1 + 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index df96f4c17..a9a908e13 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -144,7 +144,16 @@ QWebEnginePagePrivate::~QWebEnginePagePrivate() RenderWidgetHostViewQtDelegate *QWebEnginePagePrivate::CreateRenderWidgetHostViewQtDelegate(RenderWidgetHostViewQtDelegateClient *client) { - return new RenderWidgetHostViewQtDelegateWidget(client); + // Set the QWebEngineView as the parent for a popup delegate, so that the new popup window + // responds properly to clicks in case the QWebEngineView is inside a modal QDialog. Setting the + // parent essentially notifies the OS that the popup window is part of the modal session, and + // should allow interaction. + // The new delegate will not be deleted by the parent view though, because we unset the parent + // when the parent is destroyed. The delegate will be destroyed by Chromium when the popup is + // dismissed. + // If the delegate is not for a popup, but for a newly created QWebEngineView, the parent is 0 + // just like before. + return new RenderWidgetHostViewQtDelegateWidget(client, this->view); } void QWebEnginePagePrivate::titleChanged(const QString &title) diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp index 5bc1671df..a3ad898ad 100644 --- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp +++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp @@ -103,6 +103,21 @@ RenderWidgetHostViewQtDelegateWidget::RenderWidgetHostViewQtDelegateWidget(Rende setAttribute(Qt::WA_AcceptTouchEvents); setAttribute(Qt::WA_OpaquePaintEvent); setAttribute(Qt::WA_AlwaysShowToolTips); + + if (parent) { + // Unset the popup parent if the parent is being destroyed, thus making sure a double + // delete does not happen. + // Also in case the delegate is destroyed before its parent (when a popup is simply + // dismissed), this connection will automatically be removed by ~QObject(), preventing + // a use-after-free. + connect(parent, &QObject::destroyed, + this, &RenderWidgetHostViewQtDelegateWidget::removeParentBeforeParentDelete); + } +} + +void RenderWidgetHostViewQtDelegateWidget::removeParentBeforeParentDelete() +{ + setParent(Q_NULLPTR); } void RenderWidgetHostViewQtDelegateWidget::initAsChild(WebContentsAdapterClient* container) @@ -125,7 +140,11 @@ void RenderWidgetHostViewQtDelegateWidget::initAsPopup(const QRect& screenRect) // to be destroyed. setAttribute(Qt::WA_ShowWithoutActivating); setFocusPolicy(Qt::NoFocus); - setWindowFlags(Qt::ToolTip | Qt::FramelessWindowHint | Qt::WindowDoesNotAcceptFocus); + + // macOS doesn't like Qt::ToolTip when QWebEngineView is inside a modal dialog, specifically by + // not forwarding click events to the popup. So we use Qt::Tool which behaves the same way, but + // works on macOS too. + setWindowFlags(Qt::Tool | Qt::FramelessWindowHint | Qt::WindowDoesNotAcceptFocus); setGeometry(screenRect); show(); diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h index ecf2d2d33..aa9495105 100644 --- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h +++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h @@ -92,6 +92,7 @@ protected: private slots: void onWindowPosChanged(); + void removeParentBeforeParentDelete(); private: RenderWidgetHostViewQtDelegateClient *m_client; -- cgit v1.2.3 From 5fc3be56ea6c1d7b1b7567ce429774e92859dd14 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Wed, 9 Nov 2016 10:30:04 +0100 Subject: Check if the .git directory exists before invoking git If .git does not exist then this is not from a git build and therefore there is no reason to invoke git (which might not even exist on the system anyway). This prevents an error appearing due to trying to invoke git on Windows when it does not exist. Change-Id: I8c0b5b237cfdaffdbb33efdd16cf20cd1560f1a1 Reviewed-by: Joerg Bornemann --- tools/qmake/mkspecs/features/functions.prf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/qmake/mkspecs/features/functions.prf b/tools/qmake/mkspecs/features/functions.prf index a5481e2b9..2fbbb1b42 100644 --- a/tools/qmake/mkspecs/features/functions.prf +++ b/tools/qmake/mkspecs/features/functions.prf @@ -102,7 +102,7 @@ defineReplace(getConfigDir) { } defineReplace(getChromiumSrcDir) { - git_chromium_src_dir = $$system("git config qtwebengine.chromiumsrcdir") + exists($$QTWEBENGINE_ROOT/.git): git_chromium_src_dir = $$system("git config qtwebengine.chromiumsrcdir") # Fall back to the snapshot path if git does not know about chromium sources (i.e. init-repository.py has not been used) isEmpty(git_chromium_src_dir): git_chromium_src_dir = "src/3rdparty/chromium" return($$git_chromium_src_dir) -- cgit v1.2.3 From db1f77b2c44baa8426d0a5911b6c234704e9cc90 Mon Sep 17 00:00:00 2001 From: Peter Varga Date: Mon, 21 Nov 2016 14:52:05 +0100 Subject: Fix IPC message logging Register Qt IPC messages for logging. The usage of content::RegisterIPCLogger function adds the content_common.content_ipc_logging.o to the link dependency. Thus it will register the Chromium Content IPC messages too. Task-number: QTBUG-57224 Change-Id: I2c45691feb22a34f6074940cc35b8a4ba7804370 Reviewed-by: Allan Sandfeld Jensen --- src/core/common/qt_ipc_logging.cpp | 48 ++++++++++++++++++++++++++++++++++++++ src/core/core_gyp_generator.pro | 1 + 2 files changed, 49 insertions(+) create mode 100644 src/core/common/qt_ipc_logging.cpp diff --git a/src/core/common/qt_ipc_logging.cpp b/src/core/common/qt_ipc_logging.cpp new file mode 100644 index 000000000..124124de1 --- /dev/null +++ b/src/core/common/qt_ipc_logging.cpp @@ -0,0 +1,48 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtWebEngine module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED + +#if defined(IPC_MESSAGE_LOG_ENABLED) +#define IPC_MESSAGE_MACROS_LOG_ENABLED +#include "content/public/common/content_ipc_logging.h" +#define IPC_LOG_TABLE_ADD_ENTRY(msg_id, logger) \ + content::RegisterIPCLogger(msg_id, logger) +#include "common/qt_messages.h" +#endif diff --git a/src/core/core_gyp_generator.pro b/src/core/core_gyp_generator.pro index ec180d38e..4b0f6d90a 100644 --- a/src/core/core_gyp_generator.pro +++ b/src/core/core_gyp_generator.pro @@ -44,6 +44,7 @@ SOURCES = \ chromium_gpu_helper.cpp \ chromium_overrides.cpp \ clipboard_qt.cpp \ + common/qt_ipc_logging.cpp \ common/qt_messages.cpp \ common/user_script_data.cpp \ content_client_qt.cpp \ -- cgit v1.2.3 From 95435bbef3102727759e826bed63fe3d0a6ce057 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 23 Nov 2016 15:20:41 +0100 Subject: Blacklist doNotSendMouseKeyboardEventsWhenDisabled on Windows It fails occasionally and blocks integration Task-number: QTBUG-57117 Change-Id: I7266ccf4fe7e0905dc31e5e7c085be6f90d0aa03 Reviewed-by: Alexandru Croitor --- tests/auto/widgets/qwebengineview/BLACKLIST | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tests/auto/widgets/qwebengineview/BLACKLIST diff --git a/tests/auto/widgets/qwebengineview/BLACKLIST b/tests/auto/widgets/qwebengineview/BLACKLIST new file mode 100644 index 000000000..0a909d0f6 --- /dev/null +++ b/tests/auto/widgets/qwebengineview/BLACKLIST @@ -0,0 +1,2 @@ +[doNotSendMouseKeyboardEventsWhenDisabled] +windows -- cgit v1.2.3 From 95a22234a4671e6e79ce1cecbd720551a185d278 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 16 Nov 2016 13:46:56 +0100 Subject: ResourceType ABI/API fixup [ChangeLog][Important Changes] The enum value ResourceTypeUnknown has changed value since there was a mismatch between 5.6 and 5.7+ definitions. In general any unknown ResourceType value should handled as unknown for forward compatibility, since more types are and can be added in later Qt versions. Change-Id: I0a9f8a2129d4549deeae01e199f432fbbf1bbb9e Reviewed-by: Leena Miettinen Reviewed-by: Kai Koehne --- src/core/api/qwebengineurlrequestinfo.cpp | 5 ++++- src/core/api/qwebengineurlrequestinfo.h | 5 ++++- src/core/network_delegate_qt.cpp | 26 +++++++++++++++++++------- src/core/web_contents_delegate_qt.cpp | 2 +- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/core/api/qwebengineurlrequestinfo.cpp b/src/core/api/qwebengineurlrequestinfo.cpp index f229a9748..93cdf012f 100644 --- a/src/core/api/qwebengineurlrequestinfo.cpp +++ b/src/core/api/qwebengineurlrequestinfo.cpp @@ -59,7 +59,7 @@ ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeFavicon, content::RESOU ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeXhr, content::RESOURCE_TYPE_XHR) ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypePing, content::RESOURCE_TYPE_PING) ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeServiceWorker, content::RESOURCE_TYPE_SERVICE_WORKER) -ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeUnknown, content::RESOURCE_TYPE_LAST_TYPE) +ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeLast, content::RESOURCE_TYPE_LAST_TYPE) ASSERT_ENUMS_MATCH(QtWebEngineCore::WebContentsAdapterClient::LinkNavigation, QWebEngineUrlRequestInfo::NavigationTypeLink) ASSERT_ENUMS_MATCH(QtWebEngineCore::WebContentsAdapterClient::TypedNavigation, QWebEngineUrlRequestInfo::NavigationTypeTyped) @@ -166,6 +166,9 @@ QWebEngineUrlRequestInfo::QWebEngineUrlRequestInfo(QWebEngineUrlRequestInfoPriva \value ResourceTypePing A ping request for . \value ResourceTypeServiceWorker The main resource of a service worker. \value ResourceTypeUnknown Unknown request type. + + \note For forward compatibility all values not matched should be treated as unknown, + not just \c ResourceTypeUnknown. */ /*! diff --git a/src/core/api/qwebengineurlrequestinfo.h b/src/core/api/qwebengineurlrequestinfo.h index 9a13b3faf..c7af45375 100644 --- a/src/core/api/qwebengineurlrequestinfo.h +++ b/src/core/api/qwebengineurlrequestinfo.h @@ -70,7 +70,10 @@ public: ResourceTypeXhr, // a XMLHttpRequest ResourceTypePing, // a ping request for ResourceTypeServiceWorker, // the main resource of a service worker. - ResourceTypeUnknown +#ifndef Q_QDOC + ResourceTypeLast, +#endif + ResourceTypeUnknown = 255 }; enum NavigationType { diff --git a/src/core/network_delegate_qt.cpp b/src/core/network_delegate_qt.cpp index fd79917f4..e14180ae6 100644 --- a/src/core/network_delegate_qt.cpp +++ b/src/core/network_delegate_qt.cpp @@ -55,7 +55,7 @@ namespace QtWebEngineCore { -int pageTransitionToNavigationType(ui::PageTransition transition) +WebContentsAdapterClient::NavigationType pageTransitionToNavigationType(ui::PageTransition transition) { int32 qualifier = ui::PageTransitionGetQualifier(transition); @@ -78,6 +78,18 @@ int pageTransitionToNavigationType(ui::PageTransition transition) } } +QWebEngineUrlRequestInfo::ResourceType toQt(content::ResourceType resourceType) +{ + if (resourceType >= 0 && resourceType < content::ResourceType(QWebEngineUrlRequestInfo::ResourceTypeLast)) + return static_cast(resourceType); + return QWebEngineUrlRequestInfo::ResourceTypeUnknown; +} + +QWebEngineUrlRequestInfo::NavigationType toQt(WebContentsAdapterClient::NavigationType navigationType) +{ + return static_cast(navigationType); +} + NetworkDelegateQt::NetworkDelegateQt(URLRequestContextGetterQt *requestContext) : m_requestContextGetter(requestContext) { @@ -91,7 +103,7 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, const net::C const content::ResourceRequestInfo *resourceInfo = content::ResourceRequestInfo::ForRequest(request); content::ResourceType resourceType = content::RESOURCE_TYPE_LAST_TYPE; - int navigationType = QWebEngineUrlRequestInfo::NavigationTypeOther; + WebContentsAdapterClient::NavigationType navigationType = WebContentsAdapterClient::OtherNavigation; if (resourceInfo) { resourceType = resourceInfo->GetResourceType(); @@ -102,11 +114,11 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, const net::C QWebEngineUrlRequestInterceptor* interceptor = m_requestContextGetter->m_requestInterceptor; if (interceptor) { - QWebEngineUrlRequestInfoPrivate *infoPrivate = new QWebEngineUrlRequestInfoPrivate(static_cast(resourceType) - , static_cast(navigationType) - , qUrl - , toQt(request->first_party_for_cookies()) - , QByteArray::fromStdString(request->method())); + QWebEngineUrlRequestInfoPrivate *infoPrivate = new QWebEngineUrlRequestInfoPrivate(toQt(resourceType), + toQt(navigationType), + qUrl, + toQt(request->first_party_for_cookies()), + QByteArray::fromStdString(request->method())); QWebEngineUrlRequestInfo requestInfo(infoPrivate); interceptor->interceptRequest(requestInfo); if (requestInfo.changed()) { diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index 5e9157069..95a66f622 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -393,7 +393,7 @@ void WebContentsDelegateQt::requestGeolocationPermission(const QUrl &requestingO m_viewClient->runGeolocationPermissionRequest(requestingOrigin); } -extern int pageTransitionToNavigationType(ui::PageTransition transition); +extern WebContentsAdapterClient::NavigationType pageTransitionToNavigationType(ui::PageTransition transition); void WebContentsDelegateQt::launchExternalURL(const QUrl &url, ui::PageTransition page_transition, bool is_main_frame) { -- cgit v1.2.3