From 93f9eed62db271ae4b8896f48df72a956f3ce7be Mon Sep 17 00:00:00 2001
From: Viktor Engelmann <Viktor.Engelmann@qt.io>
Date: Mon, 13 Mar 2017 14:30:02 +0100
Subject: [PATCH] Store Target URL in WebContentsDelegateQt::WebContentsCreated
When opening a new window, for example by using the JavaScript
method window.open('...'), the requested url is not stored
in the content::WebContents object we get in
WebContentsDelegateQt::createWindow (at this point, it should
at least be stored as pending request in the WebContents'
NavigationController, but it is not).
Because of this, the QQuickWebEngineNewViewRequest object
in QQuickWebEngineViewPrivate::adoptNewWindow never contained
the url. We have access to the target url in
WebContentsDelegateQt::WebContentsCreated, so now we store
it there in a new property m_initialTargetUrl, from where
WebContentsDelegateQt::createWindow takes it and passes it
to WebContentsAdapter::adoptNewWindow as a new parameter.
[ChangeLog][WebEngine] Fix WebEngineNewViewRequest::requestedUrl being empty when opening window from JavaScript
Task-number: QTBUG-57675
Change-Id: I7e2c7866899baade17ce2517e6be8b2b2709699e
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
src/core/web_contents_adapter_client.h | 2 +-
src/core/web_contents_delegate_qt.cpp | 7 ++++++-
src/core/web_contents_delegate_qt.h | 2 ++
src/webengine/api/qquickwebengineview.cpp | 5 ++---
src/webengine/api/qquickwebengineview_p_p.h | 2 +-
src/webenginewidgets/api/qwebenginepage.cpp | 3 ++-
src/webenginewidgets/api/qwebenginepage_p.h | 2 +-
.../quick/qmltests/data/tst_newViewRequest.qml | 18 ++++++++++++------
8 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index d4b2974fc..055cefa6f 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -330,7 +330,7 @@ public:
virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) = 0;
virtual void focusContainer() = 0;
virtual void unhandledKeyEvent(QKeyEvent *event) = 0;
- virtual void adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry) = 0;
+ virtual void adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry, const QUrl &targetUrl) = 0;
virtual bool isBeingAdopted() = 0;
virtual void close() = 0;
virtual void windowCloseRejected() = 0;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index 6c5b63960..86366abaa 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -282,6 +282,11 @@ void WebContentsDelegateQt::DidUpdateFaviconURL(const std::vector<content::Favic
m_faviconManager->update(faviconCandidates);
}
+void WebContentsDelegateQt::WebContentsCreated(content::WebContents* /*source_contents*/, int /*opener_render_process_id*/, int /*opener_render_frame_id*/, const std::string& /*frame_name*/, const GURL& target_url, content::WebContents* new_contents)
+{
+ this->m_initialTargetUrl = toQt(target_url);
+}
+
content::ColorChooser *WebContentsDelegateQt::OpenColorChooser(content::WebContents *source, SkColor color, const std::vector<content::ColorSuggestion> &suggestion)
{
Q_UNUSED(suggestion);
@@ -422,7 +427,7 @@ QWeakPointer<WebContentsAdapter> WebContentsDelegateQt::createWindow(content::We
{
QSharedPointer<WebContentsAdapter> newAdapter = QSharedPointer<WebContentsAdapter>::create(new_contents);
- m_viewClient->adoptNewWindow(newAdapter, static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), user_gesture, toQt(initial_pos));
+ m_viewClient->adoptNewWindow(newAdapter, static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), user_gesture, toQt(initial_pos), m_initialTargetUrl);
// If the client didn't reference the adapter, it will be deleted now, and the weak pointer zeroed.
return newAdapter;
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index 2e294a841..913bf356c 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -104,6 +104,7 @@ public:
void LoadProgressChanged(content::WebContents* source, double progress) override;
void HandleKeyboardEvent(content::WebContents *source, const content::NativeWebKeyboardEvent &event) override;
content::ColorChooser *OpenColorChooser(content::WebContents *source, SkColor color, const std::vector<content::ColorSuggestion> &suggestion) override;
+ void WebContentsCreated(content::WebContents* source_contents, int opener_render_process_id, int opener_render_frame_id, const std::string& frame_name, const GURL& target_url, content::WebContents* new_contents) override;
content::JavaScriptDialogManager *GetJavaScriptDialogManager(content::WebContents *source) override;
void EnterFullscreenModeForTab(content::WebContents* web_contents, const GURL& origin) override;
void ExitFullscreenModeForTab(content::WebContents*) override;
@@ -156,6 +157,7 @@ private:
QScopedPointer<FaviconManager> m_faviconManager;
SavePageInfo m_savePageInfo;
QSharedPointer<FilePickerController> m_filePickerController;
+ QUrl m_initialTargetUrl;
};
} // namespace QtWebEngineCore
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index be86f5daf..cd734f971 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -586,7 +586,7 @@ void QQuickWebEngineViewPrivate::unhandledKeyEvent(QKeyEvent *event)
q->window()->sendEvent(q->parentItem(), event);
}
-void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &)
+void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &, const QUrl &targetUrl)
{
Q_Q(QQuickWebEngineView);
QQuickWebEngineNewViewRequest request;
@@ -594,8 +594,7 @@ void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer<WebContentsAdapte
// to start loading it and possibly return it to its parent page window.open().
request.m_adapter = newWebContents;
request.m_isUserInitiated = userGesture;
- if (newWebContents)
- request.m_requestedUrl = newWebContents->requestedUrl();
+ request.m_requestedUrl = targetUrl;
switch (disposition) {
case WebContentsAdapterClient::NewForegroundTabDisposition:
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 2ecd70d78..64c63f31a 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -107,7 +107,7 @@ public:
virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) Q_DECL_OVERRIDE;
virtual void focusContainer() Q_DECL_OVERRIDE;
virtual void unhandledKeyEvent(QKeyEvent *event) Q_DECL_OVERRIDE;
- virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &) Q_DECL_OVERRIDE;
+ virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &, const QUrl &targetUrl) Q_DECL_OVERRIDE;
virtual bool isBeingAdopted() Q_DECL_OVERRIDE;
virtual void close() Q_DECL_OVERRIDE;
virtual void windowCloseRejected() Q_DECL_OVERRIDE;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 09d05a438..416b3a5a2 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -401,10 +401,11 @@ void QWebEnginePagePrivate::unhandledKeyEvent(QKeyEvent *event)
QGuiApplication::sendEvent(view->parentWidget(), event);
}
-void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry)
+void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry, const QUrl &targetUrl)
{
Q_Q(QWebEnginePage);
Q_UNUSED(userGesture);
+ Q_UNUSED(targetUrl);
QWebEnginePage *newPage = q->createWindow(toWindowType(disposition));
if (!newPage)
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index c7b805c45..0a1fcd34e 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -100,7 +100,7 @@ public:
virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) Q_DECL_OVERRIDE;
virtual void focusContainer() Q_DECL_OVERRIDE;
virtual void unhandledKeyEvent(QKeyEvent *event) Q_DECL_OVERRIDE;
- virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry) Q_DECL_OVERRIDE;
+ virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry, const QUrl &targetUrl) Q_DECL_OVERRIDE;
void adoptNewWindowImpl(QWebEnginePage *newPage,
const QSharedPointer<QtWebEngineCore::WebContentsAdapter> &newWebContents,
const QRect &initialGeometry);
diff --git a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
index 7a04d5f5b..4becbb620 100644
--- a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
+++ b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
@@ -28,7 +28,7 @@
import QtQuick 2.0
import QtTest 1.0
-import QtWebEngine 1.2
+import QtWebEngine 1.5
TestWebEngineView {
id: webEngineView
@@ -47,7 +47,8 @@ TestWebEngineView {
onNewViewRequested: {
newViewRequest = {
"destination": request.destination,
- "userInitiated": request.userInitiated
+ "userInitiated": request.userInitiated,
+ "requestedUrl": request.requestedUrl
};
dialog = Qt.createQmlObject(
@@ -81,6 +82,8 @@ TestWebEngineView {
}
function test_jsWindowOpen() {
+ var url = 'data:text/html,%3Chtml%3E%3Cbody%3ETest+Page%3C%2Fbody%3E%3C%2Fhtml%3E';
+
// Open an empty page in a new tab
webEngineView.loadHtml(
"<html><head><script>" +
@@ -95,28 +98,30 @@ TestWebEngineView {
verify(dialog.webEngineView.waitForLoadSucceeded());
compare(dialog.webEngineView.url, "");
+ compare(newViewRequest.requestedUrl, 'about:blank');
newViewRequestedSpy.clear();
dialog.destroy();
- // Open an empty page in a new dialog
+ // Open a page in a new dialog
webEngineView.loadHtml(
"<html><head><script>" +
- " function popup() { window.open('', '_blank', 'width=200,height=100'); }" +
+ " function popup() { window.open('" + url + "', '_blank', 'width=200,height=100'); }" +
"</script></head>" +
"<body onload='popup()'></body></html>");
verify(webEngineView.waitForLoadSucceeded());
tryCompare(newViewRequestedSpy, "count", 1);
compare(newViewRequest.destination, WebEngineView.NewViewInDialog);
+ compare(newViewRequest.requestedUrl, url);
verify(!newViewRequest.userInitiated);
verify(dialog.webEngineView.waitForLoadSucceeded());
newViewRequestedSpy.clear();
dialog.destroy();
- // Open an empty page in a new dialog by user
+ // Open a page in a new dialog by user
webEngineView.loadHtml(
"<html><head><script>" +
- " function popup() { window.open('', '_blank', 'width=200,height=100'); }" +
+ " function popup() { window.open('" + url + "', '_blank', 'width=200,height=100'); }" +
"</script></head>" +
"<body onload=\"document.getElementById('popupButton').focus();\">" +
" <button id='popupButton' onclick='popup()'>Pop Up!</button>" +
@@ -124,6 +129,7 @@ TestWebEngineView {
verify(webEngineView.waitForLoadSucceeded());
verifyElementHasFocus("popupButton");
keyPress(Qt.Key_Enter);
+ compare(newViewRequest.requestedUrl, url);
tryCompare(newViewRequestedSpy, "count", 1);
compare(newViewRequest.destination, WebEngineView.NewViewInDialog);
--
GitLab