Fix new view request handling

Ignore url loading if the request is not from a data
url and the Q(Quick)WebEngineNewViewRequest.openIn() is not
called on newViewRequested().

Set the missing Q(Quick)WebEngineNewViewRequest::requestedUrl
property.

Fixes: QTBUG-87378
Change-Id: Idddc9cf075db68dcf5825b3e746d16419d02cfa0
Reviewed-by: Tamas Zakor <ztamas@inf.u-szeged.hu>
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

7 files changed, 50 insertions, 8 deletions

diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index d5a3934fe..36af1b025 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -128,7 +128,7 @@ content::WebContents *WebContentsDelegateQt::OpenURLFromTab(content::WebContents
     content::SiteInstance *target_site_instance = params.source_site_instance.get();
     content::Referrer referrer = params.referrer;
     if (params.disposition != WindowOpenDisposition::CURRENT_TAB) {
-        QSharedPointer<WebContentsAdapter> targetAdapter = createWindow(0, params.disposition, gfx::Rect(), params.user_gesture);
+        QSharedPointer<WebContentsAdapter> targetAdapter = createWindow(nullptr, params.disposition, gfx::Rect(), toQt(params.url), params.user_gesture);
         if (targetAdapter) {
             if (targetAdapter->profile() != source->GetBrowserContext()) {
                 target_site_instance = nullptr;
@@ -137,6 +137,8 @@ content::WebContents *WebContentsDelegateQt::OpenURLFromTab(content::WebContents
             if (!targetAdapter->isInitialized())
                 targetAdapter->initialize(target_site_instance);
             target = targetAdapter->webContents();
+        } else {
+            return target;
         }
     }
     Q_ASSERT(target);
@@ -235,7 +237,7 @@ QUrl WebContentsDelegateQt::url(content::WebContents* source) const {
 void WebContentsDelegateQt::AddNewContents(content::WebContents* source, std::unique_ptr<content::WebContents> new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked)
 {
     Q_UNUSED(source)
-    QSharedPointer<WebContentsAdapter> newAdapter = createWindow(std::move(new_contents), disposition, initial_pos, user_gesture);
+    QSharedPointer<WebContentsAdapter> newAdapter = createWindow(std::move(new_contents), disposition, initial_pos, m_initialTargetUrl, user_gesture);
     // Chromium can forget to pass user-agent override settings to new windows (see QTBUG-61774 and QTBUG-76249),
     // so set it here. Note the actual value doesn't really matter here. Only the second value does, but we try
     // to give the correct user-agent anyway.
@@ -675,7 +677,7 @@ void WebContentsDelegateQt::overrideWebPreferences(content::WebContents *webCont
 
 QSharedPointer<WebContentsAdapter>
 WebContentsDelegateQt::createWindow(std::unique_ptr<content::WebContents> new_contents,
-                                    WindowOpenDisposition disposition, const gfx::Rect &initial_pos,
+                                    WindowOpenDisposition disposition, const gfx::Rect &initial_pos, const QUrl &url,
                                     bool user_gesture)
 {
     QSharedPointer<WebContentsAdapter> newAdapter = QSharedPointer<WebContentsAdapter>::create(std::move(new_contents));
@@ -683,7 +685,7 @@ WebContentsDelegateQt::createWindow(std::unique_ptr<content::WebContents> new_co
     return m_viewClient->adoptNewWindow(
             std::move(newAdapter),
             static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), user_gesture,
-            toQt(initial_pos), m_initialTargetUrl);
+            toQt(initial_pos), url);
 }
 
 void WebContentsDelegateQt::allowCertificateError(const QSharedPointer<CertificateErrorController> &errorController)
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index 159803840..e9ac3e7f4 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -205,6 +205,7 @@ private:
     QSharedPointer<WebContentsAdapter>
     createWindow(std::unique_ptr<content::WebContents> new_contents,
                  WindowOpenDisposition disposition, const gfx::Rect &initial_pos,
+                 const QUrl &url,
                  bool user_gesture);
     void EmitLoadStarted(const QUrl &url, bool isErrorPage = false);
     void EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString());
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 6fb3c5c43..828a1ca79 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -337,8 +337,13 @@ QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebC
     Q_UNUSED(targetUrl);
 
     QWebEnginePage *newPage = q->createWindow(toWindowType(disposition));
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
     if (!newPage)
         return nullptr;
+#else
+    if (!newPage)
+        return adapter;
+#endif
 
     if (!newWebContents->webContents())
         return newPage->d_func()->adapter; // Reuse existing adapter
diff --git a/tests/auto/quick/qmltests/data/TestWebEngineView.qml b/tests/auto/quick/qmltests/data/TestWebEngineView.qml
index 6db076ae8..f2bc09e4b 100644
--- a/tests/auto/quick/qmltests/data/TestWebEngineView.qml
+++ b/tests/auto/quick/qmltests/data/TestWebEngineView.qml
@@ -85,12 +85,14 @@ WebEngineView {
 
     function getElementCenter(element) {
             var center;
-            runJavaScript("(function() {" +
+            testCase.tryVerify(function() {
+                runJavaScript("(function() {" +
                           "   var elem = document.getElementById('" + element + "');" +
                           "   var rect = elem.getBoundingClientRect();" +
                           "   return { 'x': (rect.left + rect.right) / 2, 'y': (rect.top + rect.bottom) / 2 };" +
                           "})();", function(result) { center = result } );
-            testCase.tryVerify(function() { return center !== undefined; });
+                return center !== undefined;
+            });
             return center;
     }
 
diff --git a/tests/auto/quick/qmltests/data/test2.html b/tests/auto/quick/qmltests/data/test2.html
index 629c2a063..7a02bf1f2 100644
--- a/tests/auto/quick/qmltests/data/test2.html
+++ b/tests/auto/quick/qmltests/data/test2.html
@@ -1,6 +1,6 @@
 <html>
 <head><title>Test page with huge link area</title></head>
 <body>
-<a title="A title" href="test1.html"><img width=200 height=200></a>
+<a id="link" title="A title" href="test1.html"><img width=200 height=200></a>
 </body>
 </html>
diff --git a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
index 80389e9f8..08d63d956 100644
--- a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
+++ b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
@@ -38,6 +38,13 @@ TestWebEngineView {
     property var newViewRequest: null
     property var dialog: null
     property string viewType: ""
+    property var loadRequestArray: []
+
+    onLoadingChanged: {
+        loadRequestArray.push({
+            "status": loadRequest.status,
+        });
+    }
 
     SignalSpy {
         id: newViewRequestedSpy
@@ -81,6 +88,7 @@ TestWebEngineView {
             newViewRequestedSpy.clear();
             newViewRequest = null;
             viewType = "";
+            loadRequestArray = [];
         }
 
         function cleanup() {
@@ -163,6 +171,23 @@ TestWebEngineView {
                 }
                 newViewRequestedSpy.clear();
             }
+
+            loadRequestArray = [];
+            compare(loadRequestArray.length, 0);
+            webEngineView.url = Qt.resolvedUrl("test2.html");
+            verify(webEngineView.waitForLoadSucceeded());
+            var center = getElementCenter("link");
+            mouseClick(webEngineView, center.x, center.y, Qt.LeftButton, Qt.ControlModifier);
+            tryCompare(newViewRequestedSpy, "count", 1);
+            compare(newViewRequest.requestedUrl, Qt.resolvedUrl("test1.html"));
+            compare(newViewRequest.destination, WebEngineView.NewViewInBackgroundTab);
+            verify(newViewRequest.userInitiated);
+            if (viewType === "" || viewType === "null") {
+                compare(loadRequestArray[0].status, WebEngineView.LoadStartedStatus);
+                compare(loadRequestArray[1].status, WebEngineView.LoadSucceededStatus);
+                compare(loadRequestArray.length, 2);
+            }
+            newViewRequestedSpy.clear();
         }
     }
 }
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index 040114258..55e888abf 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -3458,7 +3458,11 @@ void tst_QWebEnginePage::openLinkInNewPage_data()
     // the disposition and performing the navigation request normally.
 
     QTest::newRow("BlockPopup")     << Decision::ReturnNull  << Cause::TargetBlank << Effect::Blocked;
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
+    QTest::newRow("IgnoreIntent")   << Decision::ReturnNull  << Cause::MiddleClick << Effect::Blocked;
+#else
     QTest::newRow("IgnoreIntent")   << Decision::ReturnNull  << Cause::MiddleClick << Effect::LoadInSelf;
+#endif
     QTest::newRow("OverridePopup")  << Decision::ReturnSelf  << Cause::TargetBlank << Effect::LoadInSelf;
     QTest::newRow("OverrideIntent") << Decision::ReturnSelf  << Cause::MiddleClick << Effect::LoadInSelf;
     QTest::newRow("AcceptPopup")    << Decision::ReturnOther << Cause::TargetBlank << Effect::LoadInOther;
@@ -3535,7 +3539,10 @@ void tst_QWebEnginePage::openLinkInNewPage()
 
     switch (effect) {
     case Effect::Blocked:
-        // Nothing to test
+        // Test nothing new loaded
+        QTest::qWait(500);
+        QCOMPARE(page1.spy.count(), 0);
+        QCOMPARE(page2.spy.count(), 0);
         break;
     case Effect::LoadInSelf:
         QTRY_COMPARE(page1.spy.count(), 1);