summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/webenginewidgets/api/qwebenginepage.cpp19
-rw-r--r--src/webenginewidgets/api/qwebenginepage_p.h3
-rw-r--r--tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp158
3 files changed, 158 insertions, 22 deletions
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index b267c5dd1..6f0009183 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -358,25 +358,6 @@ void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> ne
if (!newPage)
return;
- if (newPage->d_func() == this) {
- // If createWindow returns /this/ we must delay the adoption.
- Q_ASSERT(q == newPage);
- // WebContents might be null if we just opened a new page for navigation, in that case
- // avoid referencing newWebContents so that it is deleted and WebContentsDelegateQt::OpenURLFromTab
- // will fall back to navigating current page.
- if (newWebContents->webContents()) {
- QTimer::singleShot(0, q, [this, newPage, newWebContents, initialGeometry] () {
- adoptNewWindowImpl(newPage, newWebContents, initialGeometry);
- });
- }
- } else {
- adoptNewWindowImpl(newPage, newWebContents, initialGeometry);
- }
-}
-
-void QWebEnginePagePrivate::adoptNewWindowImpl(QWebEnginePage *newPage,
- const QSharedPointer<WebContentsAdapter> &newWebContents, const QRect &initialGeometry)
-{
// Mark the new page as being in the process of being adopted, so that a second mouse move event
// sent by newWebContents->initialize() gets filtered in RenderWidgetHostViewQt::forwardEvent.
// The first mouse move event is being sent by q->createWindow(). This is necessary because
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index f37413b8e..ae845c530 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -113,9 +113,6 @@ public:
void focusContainer() override;
void unhandledKeyEvent(QKeyEvent *event) override;
void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry, const QUrl &targetUrl) override;
- void adoptNewWindowImpl(QWebEnginePage *newPage,
- const QSharedPointer<QtWebEngineCore::WebContentsAdapter> &newWebContents,
- const QRect &initialGeometry);
bool isBeingAdopted() override;
void close() override;
void windowCloseRejected() override;
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index a6b3e0905..a638b7183 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -198,6 +198,8 @@ private Q_SLOTS:
void dataURLFragment();
void devTools();
void openLinkInDifferentProfile();
+ void openLinkInNewPage_data();
+ void openLinkInNewPage();
void triggerActionWithoutMenu();
void dynamicFrame();
@@ -3379,6 +3381,162 @@ void tst_QWebEnginePage::openLinkInDifferentProfile()
QVERIFY(spy2.takeFirst().value(0).toBool());
}
+// What does createWindow do?
+enum class OpenLinkInNewPageDecision {
+ // Returns nullptr,
+ ReturnNull,
+ // Returns this,
+ ReturnSelf,
+ // Returns page != this
+ ReturnOther,
+};
+
+// What causes createWindow to be called?
+enum class OpenLinkInNewPageCause {
+ // User clicks on a link with target=_blank.
+ TargetBlank,
+ // User clicks with MiddleButton.
+ MiddleClick,
+};
+
+// What happens after createWindow?
+enum class OpenLinkInNewPageEffect {
+ // The navigation request disappears into the ether.
+ Blocked,
+ // The navigation request becomes a navigation in the original page.
+ LoadInSelf,
+ // The navigation request becomes a navigation in a different page.
+ LoadInOther,
+};
+
+Q_DECLARE_METATYPE(OpenLinkInNewPageCause)
+Q_DECLARE_METATYPE(OpenLinkInNewPageDecision)
+Q_DECLARE_METATYPE(OpenLinkInNewPageEffect)
+
+void tst_QWebEnginePage::openLinkInNewPage_data()
+{
+ using Decision = OpenLinkInNewPageDecision;
+ using Cause = OpenLinkInNewPageCause;
+ using Effect = OpenLinkInNewPageEffect;
+
+ QTest::addColumn<Decision>("decision");
+ QTest::addColumn<Cause>("cause");
+ QTest::addColumn<Effect>("effect");
+
+ // Note that the meaning of returning nullptr from createWindow is not
+ // consistent between the TargetBlank and MiddleClick scenarios.
+ //
+ // With TargetBlank, the open-in-new-page disposition comes from the HTML
+ // target attribute; something the user is probably not aware of. Returning
+ // nullptr is interpreted as a decision by the app to block an unwanted
+ // popup.
+ //
+ // With MiddleClick, the open-in-new-page disposition comes from the user's
+ // explicit intent. Returning nullptr is then interpreted as a failure by
+ // the app to fulfill this intent, which we try to compensate by ignoring
+ // the disposition and performing the navigation request normally.
+
+ QTest::newRow("BlockPopup") << Decision::ReturnNull << Cause::TargetBlank << Effect::Blocked;
+ QTest::newRow("IgnoreIntent") << Decision::ReturnNull << Cause::MiddleClick << Effect::LoadInSelf;
+ 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;
+ QTest::newRow("AcceptIntent") << Decision::ReturnOther << Cause::MiddleClick << Effect::LoadInOther;
+}
+
+void tst_QWebEnginePage::openLinkInNewPage()
+{
+ using Decision = OpenLinkInNewPageDecision;
+ using Cause = OpenLinkInNewPageCause;
+ using Effect = OpenLinkInNewPageEffect;
+
+ class Page : public QWebEnginePage
+ {
+ public:
+ Page *targetPage = nullptr;
+ QSignalSpy spy{this, &QWebEnginePage::loadFinished};
+ Page(QWebEngineProfile *profile) : QWebEnginePage(profile) {}
+ private:
+ QWebEnginePage *createWindow(WebWindowType) override { return targetPage; }
+ };
+
+ class View : public QWebEngineView
+ {
+ public:
+ View(Page *page)
+ {
+ resize(500, 500);
+ setPage(page);
+ }
+ };
+
+ QFETCH(Decision, decision);
+ QFETCH(Cause, cause);
+ QFETCH(Effect, effect);
+
+ QWebEngineProfile profile;
+ Page page1(&profile);
+ Page page2(&profile);
+ View view1(&page1);
+ View view2(&page2);
+
+ view1.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&view1));
+
+ page1.setHtml("<html><body>"
+ "<a id='link' href='data:,hello' target='_blank'>link</a>"
+ "</body></html>");
+ QTRY_COMPARE(page1.spy.count(), 1);
+ QVERIFY(page1.spy.takeFirst().value(0).toBool());
+
+ switch (decision) {
+ case Decision::ReturnNull:
+ page1.targetPage = nullptr;
+ break;
+ case Decision::ReturnSelf:
+ page1.targetPage = &page1;
+ break;
+ case Decision::ReturnOther:
+ page1.targetPage = &page2;
+ break;
+ }
+
+ Qt::MouseButton button;
+ switch (cause) {
+ case Cause::TargetBlank:
+ button = Qt::LeftButton;
+ break;
+ case Cause::MiddleClick:
+ button = Qt::MiddleButton;
+ break;
+ }
+ QTest::mouseClick(view1.focusProxy(), button, {}, elementCenter(&page1, "link"));
+
+ switch (effect) {
+ case Effect::Blocked:
+ // Nothing to test
+ break;
+ case Effect::LoadInSelf:
+ QTRY_COMPARE(page1.spy.count(), 1);
+ QVERIFY(page1.spy.takeFirst().value(0).toBool());
+ QCOMPARE(page2.spy.count(), 0);
+ if (decision == Decision::ReturnSelf)
+ // History was discarded
+ QCOMPARE(page1.history()->count(), 1);
+ else
+ QCOMPARE(page1.history()->count(), 2);
+ QCOMPARE(page2.history()->count(), 0);
+ break;
+ case Effect::LoadInOther:
+ QTRY_COMPARE(page2.spy.count(), 1);
+ QVERIFY(page2.spy.takeFirst().value(0).toBool());
+ QCOMPARE(page1.spy.count(), 0);
+ QCOMPARE(page1.history()->count(), 1);
+ QCOMPARE(page2.history()->count(), 1);
+ break;
+ }
+}
+
void tst_QWebEnginePage::triggerActionWithoutMenu()
{
// Calling triggerAction should not crash even when for