From 84ef53a95ac48e7043337fecc0fc4714a3911c60 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 15 Mar 2022 15:00:15 +0100 Subject: Avoid crashing on context free web actions If a web action that depends on a context is called, do nothing instead of crashing. Fixes: QTBUG-101724 Change-Id: I7a0040a58e9dc7fa14f0cae71cf1bcd086b61c54 Reviewed-by: Kirill Burtsev (cherry picked from commit e8e381119ca5b08add50d13ab9d275d9ab55471c) Reviewed-by: Qt Cherry-pick Bot --- src/core/api/qwebenginepage.cpp | 30 ++++++++++---------- tests/auto/core/devtools/tst_devtools.cpp | 7 +++++ .../widgets/qwebengineview/tst_qwebengineview.cpp | 32 ++++++++++++++++++++++ 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/src/core/api/qwebenginepage.cpp b/src/core/api/qwebenginepage.cpp index 8e03aea0d..bd6b4b1ad 100644 --- a/src/core/api/qwebenginepage.cpp +++ b/src/core/api/qwebenginepage.cpp @@ -1286,26 +1286,26 @@ void QWebEnginePage::triggerAction(WebAction action, bool) d->adapter->unselect(); break; case OpenLinkInThisWindow: - if (d->view && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) setUrl(d->view->lastContextMenuRequest()->filteredLinkUrl()); break; case OpenLinkInNewWindow: - if (d->view && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) d->createNewWindow(WebContentsAdapterClient::NewWindowDisposition, true, d->view->lastContextMenuRequest()->filteredLinkUrl()); break; case OpenLinkInNewTab: - if (d->view && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) d->createNewWindow(WebContentsAdapterClient::NewForegroundTabDisposition, true, d->view->lastContextMenuRequest()->filteredLinkUrl()); break; case OpenLinkInNewBackgroundTab: - if (d->view && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) d->createNewWindow(WebContentsAdapterClient::NewBackgroundTabDisposition, true, d->view->lastContextMenuRequest()->filteredLinkUrl()); break; case CopyLinkToClipboard: - if (d->view && !d->view->lastContextMenuRequest()->linkUrl().isEmpty()) { + if (d->view && d->view->lastContextMenuRequest() && !d->view->lastContextMenuRequest()->linkUrl().isEmpty()) { QString urlString = d->view->lastContextMenuRequest()->linkUrl().toString( QUrl::FullyEncoded); QString linkText = d->view->lastContextMenuRequest()->linkText().toHtmlEscaped(); @@ -1322,7 +1322,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case DownloadLinkToDisk: - if (d->view && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->filteredLinkUrl().isValid()) d->adapter->download(d->view->lastContextMenuRequest()->filteredLinkUrl(), d->view->lastContextMenuRequest()->suggestedFileName(), d->view->lastContextMenuRequest()->referrerUrl(), @@ -1330,7 +1330,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) break; case CopyImageToClipboard: - if (d->view && d->view->lastContextMenuRequest()->hasImageContent() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->hasImageContent() && (d->view->lastContextMenuRequest()->mediaType() == QWebEngineContextMenuRequest::MediaTypeImage || d->view->lastContextMenuRequest()->mediaType() @@ -1339,7 +1339,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case CopyImageUrlToClipboard: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && d->view->lastContextMenuRequest()->mediaType() == QWebEngineContextMenuRequest::MediaTypeImage) { QString urlString = @@ -1360,14 +1360,14 @@ void QWebEnginePage::triggerAction(WebAction action, bool) break; case DownloadImageToDisk: case DownloadMediaToDisk: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid()) + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid()) d->adapter->download(d->view->lastContextMenuRequest()->mediaUrl(), d->view->lastContextMenuRequest()->suggestedFileName(), d->view->lastContextMenuRequest()->referrerUrl(), d->view->lastContextMenuRequest()->referrerPolicy()); break; case CopyMediaUrlToClipboard: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && (d->view->lastContextMenuRequest()->mediaType() == QWebEngineContextMenuRequest::MediaTypeAudio || d->view->lastContextMenuRequest()->mediaType() @@ -1391,7 +1391,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case ToggleMediaControls: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && d->view->lastContextMenuRequest()->mediaFlags() & QWebEngineContextMenuRequest::MediaCanToggleControls) { bool enable = !(d->view->lastContextMenuRequest()->mediaFlags() @@ -1401,7 +1401,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case ToggleMediaLoop: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && (d->view->lastContextMenuRequest()->mediaType() == QWebEngineContextMenuRequest::MediaTypeAudio || d->view->lastContextMenuRequest()->mediaType() @@ -1413,7 +1413,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case ToggleMediaPlayPause: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && (d->view->lastContextMenuRequest()->mediaType() == QWebEngineContextMenuRequest::MediaTypeAudio || d->view->lastContextMenuRequest()->mediaType() @@ -1425,7 +1425,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case ToggleMediaMute: - if (d->view && d->view->lastContextMenuRequest()->mediaUrl().isValid() + if (d->view && d->view->lastContextMenuRequest() && d->view->lastContextMenuRequest()->mediaUrl().isValid() && d->view->lastContextMenuRequest()->mediaFlags() & QWebEngineContextMenuRequest::MediaHasAudio) { // Make sure to negate the value, so that toggling actually works. @@ -1436,7 +1436,7 @@ void QWebEnginePage::triggerAction(WebAction action, bool) } break; case InspectElement: - if (d->view) + if (d->view && d->view->lastContextMenuRequest()) d->adapter->inspectElementAt(d->view->lastContextMenuRequest()->position()); break; case ExitFullScreen: diff --git a/tests/auto/core/devtools/tst_devtools.cpp b/tests/auto/core/devtools/tst_devtools.cpp index 3026b3931..064f95028 100644 --- a/tests/auto/core/devtools/tst_devtools.cpp +++ b/tests/auto/core/devtools/tst_devtools.cpp @@ -48,6 +48,9 @@ void tst_DevTools::attachAndDestroyPageFirst() page->load(QUrl("data:text/plain,foobarbaz")); QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, 12000); + // shouldn't do anything until page is set + page->triggerAction(QWebEnginePage::InspectElement); + inspector->setInspectedPage(page); page->triggerAction(QWebEnginePage::InspectElement); @@ -62,6 +65,10 @@ void tst_DevTools::attachAndDestroyInspectorFirst() { // External inspector + manual destruction of inspector first QWebEnginePage* page = new QWebEnginePage(); + + // shouldn't do anything until page is set + page->triggerAction(QWebEnginePage::InspectElement); + QWebEnginePage* inspector = new QWebEnginePage(); inspector->setInspectedPage(page); diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp index 388cd3c96..3a6b799f6 100644 --- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp +++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp @@ -173,6 +173,7 @@ private Q_SLOTS: void setViewPreservesExplicitPage(); void closeDiscardsPage(); void loadAfterRendererCrashed(); + void inspectElement(); }; // This will be called before the first test function is executed. @@ -3481,5 +3482,36 @@ void tst_QWebEngineView::loadAfterRendererCrashed() QVERIFY(loadSpy.first().first().toBool()); } +void tst_QWebEngineView::inspectElement() +{ + QWebEngineView view; + view.resize(640, 480); + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + auto page = view.page(); + // shouldn't do anything until page is set + page->triggerAction(QWebEnginePage::InspectElement); + QTest::qWait(100); + + QSignalSpy spy(&view, &QWebEngineView::loadFinished); + view.load(QUrl("data:text/plain,foobarbaz")); + QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, 12000); + + // shouldn't do anything since inspector is not attached + page->triggerAction(QWebEnginePage::InspectElement); + QTest::qWait(100); + + QWebEngineView inspectorView; + inspectorView.resize(640, 480); + inspectorView.show(); + QVERIFY(QTest::qWaitForWindowExposed(&inspectorView)); + inspectorView.page()->setInspectedPage(page); + + page->triggerAction(QWebEnginePage::InspectElement); + // TODO verify somehow + QTest::qWait(100); +} + QTEST_MAIN(tst_QWebEngineView) #include "tst_qwebengineview.moc" -- cgit v1.2.3