From 3c34f95ad4a96737f66ab4933edb9e00abecdc24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Tue, 2 Jul 2019 12:36:34 +0200 Subject: Disable Cut/Copy/Unselect actions when there's no selection Fixes: QTBUG-76666 Change-Id: I74b9a26cd7be9a830f4eecd36db69777412ab316 Reviewed-by: Peter Varga --- src/webengine/api/qquickwebengineview.cpp | 9 ++++- src/webengine/api/qquickwebengineview_p_p.h | 2 +- src/webenginewidgets/api/qwebenginepage.cpp | 9 +++-- tests/auto/quick/qmltests/data/tst_action.qml | 16 ++++----- .../widgets/qwebenginepage/tst_qwebenginepage.cpp | 39 ++++++++++++++++++++++ 5 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp index 583678aaf..60db2fcd5 100644 --- a/src/webengine/api/qquickwebengineview.cpp +++ b/src/webengine/api/qquickwebengineview.cpp @@ -413,6 +413,11 @@ void QQuickWebEngineViewPrivate::didUpdateTargetURL(const QUrl &hoveredUrl) Q_EMIT q->linkHovered(hoveredUrl); } +void QQuickWebEngineViewPrivate::selectionChanged() +{ + updateEditActions(); +} + void QQuickWebEngineViewPrivate::recentlyAudibleChanged(bool recentlyAudible) { Q_Q(QQuickWebEngineView); @@ -963,12 +968,14 @@ void QQuickWebEngineViewPrivate::updateAction(QQuickWebEngineView::WebAction act break; case QQuickWebEngineView::Cut: case QQuickWebEngineView::Copy: + case QQuickWebEngineView::Unselect: + enabled = adapter->hasFocusedFrame() && !adapter->selectedText().isEmpty(); + break; case QQuickWebEngineView::Paste: case QQuickWebEngineView::Undo: case QQuickWebEngineView::Redo: case QQuickWebEngineView::SelectAll: case QQuickWebEngineView::PasteAndMatchStyle: - case QQuickWebEngineView::Unselect: enabled = adapter->hasFocusedFrame(); break; default: diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h index 1e0ba309c..e0f2595ec 100644 --- a/src/webengine/api/qquickwebengineview_p_p.h +++ b/src/webengine/api/qquickwebengineview_p_p.h @@ -109,7 +109,7 @@ public: void iconChanged(const QUrl&) override; void loadProgressChanged(int progress) override; void didUpdateTargetURL(const QUrl&) override; - void selectionChanged() override { } + void selectionChanged() override; void recentlyAudibleChanged(bool recentlyAudible) override; QRectF viewportRect() const override; QColor backgroundColor() const override; diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index 4658a4739..6e517c6c8 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -260,7 +260,10 @@ void QWebEnginePagePrivate::didUpdateTargetURL(const QUrl &hoveredUrl) void QWebEnginePagePrivate::selectionChanged() { Q_Q(QWebEnginePage); - QTimer::singleShot(0, q, &QWebEnginePage::selectionChanged); + QTimer::singleShot(0, q, [this, q]() { + updateEditActions(); + Q_EMIT q->selectionChanged(); + }); } void QWebEnginePagePrivate::recentlyAudibleChanged(bool recentlyAudible) @@ -594,12 +597,14 @@ void QWebEnginePagePrivate::updateAction(QWebEnginePage::WebAction action) const break; case QWebEnginePage::Cut: case QWebEnginePage::Copy: + case QWebEnginePage::Unselect: + enabled = adapter->hasFocusedFrame() && !adapter->selectedText().isEmpty(); + break; case QWebEnginePage::Paste: case QWebEnginePage::Undo: case QWebEnginePage::Redo: case QWebEnginePage::SelectAll: case QWebEnginePage::PasteAndMatchStyle: - case QWebEnginePage::Unselect: enabled = adapter->hasFocusedFrame(); break; default: diff --git a/tests/auto/quick/qmltests/data/tst_action.qml b/tests/auto/quick/qmltests/data/tst_action.qml index 56a91d8d0..852d4145a 100644 --- a/tests/auto/quick/qmltests/data/tst_action.qml +++ b/tests/auto/quick/qmltests/data/tst_action.qml @@ -51,8 +51,8 @@ TestWebEngineView { { webAction: WebEngineView.Forward, text: "Forward", iconName: "go-next", enabled: false }, { webAction: WebEngineView.Stop, text: "Stop", iconName: "", enabled: false }, { webAction: WebEngineView.Reload, text: "Reload", iconName: "view-refresh", enabled: true }, - { webAction: WebEngineView.Cut, text: "Cut", iconName: "Cut", enabled: true }, - { webAction: WebEngineView.Copy, text: "Copy", iconName: "", enabled: true }, + { webAction: WebEngineView.Cut, text: "Cut", iconName: "Cut", enabled: false }, + { webAction: WebEngineView.Copy, text: "Copy", iconName: "", enabled: false }, { webAction: WebEngineView.Paste, text: "Paste", iconName: "", enabled: true }, { webAction: WebEngineView.Undo, text: "Undo", iconName: "", enabled: true }, { webAction: WebEngineView.Redo, text: "Redo", iconName: "", enabled: true }, @@ -76,7 +76,7 @@ TestWebEngineView { { webAction: WebEngineView.InspectElement, text: "Inspect", iconName: "", enabled: true }, { webAction: WebEngineView.ExitFullScreen, text: "Exit full screen", iconName: "", enabled: true }, { webAction: WebEngineView.RequestClose, text: "Close Page", iconName: "", enabled: true }, - { webAction: WebEngineView.Unselect, text: "Unselect", iconName: "", enabled: true }, + { webAction: WebEngineView.Unselect, text: "Unselect", iconName: "", enabled: false }, { webAction: WebEngineView.SavePage, text: "Save page", iconName: "", enabled: true }, { webAction: WebEngineView.ViewSource, text: "View page source", iconName: "view-source", enabled: true }, { webAction: WebEngineView.ToggleBold, text: "&Bold", iconName: "", enabled: true }, @@ -110,17 +110,17 @@ TestWebEngineView { webEngineView.url = Qt.resolvedUrl("test1.html"); verify(webEngineView.waitForLoadSucceeded()); - var copyAction = webEngineView.action(WebEngineView.Copy); - verify(copyAction); + var selectAction = webEngineView.action(WebEngineView.SelectAll); + verify(selectAction); var stopAction = webEngineView.action(WebEngineView.Stop); verify(stopAction); - var triggerSpy = createTemporaryObject(signalSpy, actionTests, {target: copyAction, signalName: "triggered"}); + var triggerSpy = createTemporaryObject(signalSpy, actionTests, {target: selectAction, signalName: "triggered"}); var stopTriggerSpy = createTemporaryObject(signalSpy, actionTests, {target: stopAction, signalName: "triggered"}); - verify(copyAction.enabled); - copyAction.trigger(); + verify(selectAction.enabled); + selectAction.trigger(); compare(triggerSpy.count, 1); verify(!stopAction.enabled); diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp index e579879f4..1c9e6c7b1 100644 --- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp @@ -218,6 +218,7 @@ private Q_SLOTS: void editActionsWithExplicitFocus(); void editActionsWithInitialFocus(); void editActionsWithFocusOnIframe(); + void editActionsWithoutSelection(); private: static QPoint elementCenter(QWebEnginePage *page, const QString &id); @@ -4141,6 +4142,44 @@ void tst_QWebEnginePage::editActionsWithFocusOnIframe() QCOMPARE(page->selectedText(), QStringLiteral("inner")); } +void tst_QWebEnginePage::editActionsWithoutSelection() +{ + QWebEngineView view; + QWebEnginePage *page = view.page(); + view.settings()->setAttribute(QWebEngineSettings::FocusOnNavigationEnabled, true); + + QSignalSpy loadFinishedSpy(page, &QWebEnginePage::loadFinished); + QSignalSpy selectionChangedSpy(page, &QWebEnginePage::selectionChanged); + QSignalSpy actionChangedSpy(page->action(QWebEnginePage::SelectAll), &QAction::changed); + + page->setHtml(QString("
foo bar
")); + QTRY_COMPARE(loadFinishedSpy.count(), 1); + QTRY_COMPARE(actionChangedSpy.count(), 1); + + QVERIFY(!page->action(QWebEnginePage::Cut)->isEnabled()); + QVERIFY(!page->action(QWebEnginePage::Copy)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Paste)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Undo)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Redo)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::SelectAll)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::PasteAndMatchStyle)->isEnabled()); + QVERIFY(!page->action(QWebEnginePage::Unselect)->isEnabled()); + + page->triggerAction(QWebEnginePage::SelectAll); + QTRY_COMPARE(selectionChangedSpy.count(), 1); + QCOMPARE(page->hasSelection(), true); + QCOMPARE(page->selectedText(), QStringLiteral("foo bar")); + + QVERIFY(page->action(QWebEnginePage::Cut)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Copy)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Paste)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Undo)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Redo)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::SelectAll)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::PasteAndMatchStyle)->isEnabled()); + QVERIFY(page->action(QWebEnginePage::Unselect)->isEnabled()); +} + static QByteArrayList params = {QByteArrayLiteral("--use-fake-device-for-media-stream")}; W_QTEST_MAIN(tst_QWebEnginePage, params) -- cgit v1.2.3