diff options
| -rw-r--r-- | src/messageview.cpp | 52 | ||||
| -rw-r--r-- | src/messageview.h | 21 | ||||
| -rw-r--r-- | tests/test_messageview.cpp | 80 |
3 files changed, 153 insertions, 0 deletions
diff --git a/src/messageview.cpp b/src/messageview.cpp index bdf1b13..68821d8 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -612,6 +612,54 @@ SearchOffer MessageView::selectionSearchOffer(const QString &selectedText) const query }; } +void MessageView::removeBrowserActions(QMenu *menu, QWebEnginePage *page) +{ + if (!menu || !page) + return; + + // Item 100. Every one of these needs a history, a network or a file, and + // this pane has none of the three. + // + // ViewSource is NOT in this list, and that is deliberate. It was removed + // here first, on the reasoning that it was the same kind of thing; it is + // not. The four below have nothing to act on, while view-source has a real + // document and a real use. Chromium's own entry cannot work here either + // (it navigates to view-source:<url>, which MessagePage refuses), so item + // 113 implements it as our own plain-text dialog. Removing it in the + // meantime would delete the gesture the user reaches for. + static constexpr QWebEnginePage::WebAction kUnwanted[] = { + QWebEnginePage::Back, + QWebEnginePage::Forward, + QWebEnginePage::Reload, + QWebEnginePage::SavePage, + }; + + for (const QWebEnginePage::WebAction which : kUnwanted) { + // pageAction() is the same QAction instance the standard menu holds, + // so the pointer identifies it whatever language it is displayed in. + if (QAction *action = page->action(which)) + menu->removeAction(action); + } + + // Removing entries can leave a separator at an edge or two in a row, which + // reads as a menu that lost something. Qt has no "tidy separators", so + // this walks what is left. + const QList<QAction *> remaining = menu->actions(); + bool previousWasSeparator = true; // leading separators are unwanted too + for (QAction *action : remaining) { + if (!action->isSeparator()) { + previousWasSeparator = false; + continue; + } + if (previousWasSeparator) + menu->removeAction(action); + else + previousWasSeparator = true; + } + if (!menu->actions().isEmpty() && menu->actions().constLast()->isSeparator()) + menu->removeAction(menu->actions().constLast()); +} + void MessageView::showBodyContextMenu(const QPoint &pos) { // The page's own menu first: copy, select all and the rest stay exactly as @@ -621,6 +669,10 @@ void MessageView::showBodyContextMenu(const QPoint &pos) menu = new QMenu(this); menu->setAttribute(Qt::WA_DeleteOnClose); + // ...minus the browser's own navigation and page actions, which cannot + // apply here. Item 100. + removeBrowserActions(menu, m_view->page()); + // selectedText() reads the selection out of the render process with no // script injection. JavaScript is disabled in this profile and stays so. const SearchOffer offer = selectionSearchOffer(m_view->page()->selectedText()); diff --git a/src/messageview.h b/src/messageview.h index 56925b2..df20e40 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -29,6 +29,7 @@ class QLabel; class QMenu; +class QWebEnginePage; class QPushButton; class QWebEngineView; class QWebEngineProfile; @@ -149,6 +150,26 @@ public: /// of which notmuch reports as an error. SearchOffer selectionSearchOffer(const QString &selectedText) const; + /// Strips the browser actions out of Chromium's standard context menu. + /// + /// Item 100. The pane is not a browser: every document arrives through + /// setHtml() with a fixed base URL, so Back, Forward, Reload and Save page + /// have nothing to act on and the interceptor blocks everything by default + /// anyway. Copy and Select all are the reason the standard menu is used at + /// all, so the menu is filtered, not rebuilt. + /// + /// View source is NOT filtered, though it was at first. It has a real + /// document and a real use; item 113 implements it as our own dialog, + /// since Chromium's entry navigates to view-source:<url> and MessagePage + /// refuses that. + /// + /// Matches on the page's own QAction POINTERS, never on text, which is + /// translated and would make the filter fail in every locale but one. + /// + /// Static and taking the menu so a test can build one and check it + /// without a rendered document or a shown popup. + static void removeBrowserActions(QMenu *menu, QWebEnginePage *page); + /// Tells the pane whether the query bar currently holds anything. /// /// The menus need it to grey out "Exclude from search": excluding from an diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index c97a216..87234aa 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -17,8 +17,10 @@ */ #include <QLabel> +#include <QMenu> #include <QPushButton> #include <QSignalSpy> +#include <QWebEnginePage> #include <QWebEngineUrlScheme> #include <QWebEngineView> #include <QtTest> @@ -54,6 +56,7 @@ private slots: void headerOffersNoSenderForARealThread(); void headerOffersNothingForAnAbsentField(); void bodySelectionBecomesAQuotedSearch(); + void theBodyMenuDropsTheBrowsersOwnActions(); void aSearchFromTheDetailsDialogClosesIt(); private: @@ -699,6 +702,83 @@ void TestMessageView::bodySelectionBecomesAQuotedSearch() .label.isEmpty()); } +void TestMessageView::theBodyMenuDropsTheBrowsersOwnActions() +{ + // Item 100. The pane starts from Chromium's standard context menu, which + // is built for a browser: Back, Forward, Reload, Save page and View source + // all arrive with it and none of them can apply, since every document + // comes through setHtml() with a fixed base URL and the interceptor blocks + // everything by default. + // + // Asserted on the ACTION POINTERS, which is also how the production code + // matches them. Matching on text would pass here and fail in every locale + // but English, and an untranslated match is exactly the defect this could + // reintroduce without any test noticing. + MessageView view; + auto *page = view.findChild<QWebEnginePage *>(); + QVERIFY2(page, "no page, so this test would assert nothing"); + + QMenu menu; + const QList<QWebEnginePage::WebAction> unwanted = { + QWebEnginePage::Back, QWebEnginePage::Forward, + QWebEnginePage::Reload, QWebEnginePage::SavePage, + }; + // Kept, and the reason the standard menu is used at all rather than being + // rebuilt from scratch. + // + // ViewSource is in this list deliberately. It was removed with the four + // above at first, which was an overreach: it has a real document and a + // real use, and item 113 implements it properly. A test asserting it is + // GONE would lock in the overreach, so it asserts it survives. + // + // SelectAll is deliberately NOT here, and the reason is a limit of this + // test worth stating. This menu is built BY HAND, so "SelectAll survives" + // would only prove the filter does not remove it, and prove nothing about + // whether Chromium's real menu ever offers it. Measured by hand on + // 2026-08-17, with a selection active: the real menu holds Copy and the + // search entries and no Select all, both before and after this filter + // existed. Asserting on it here would read as a guarantee the code does + // not make. See item 117. + const QList<QWebEnginePage::WebAction> wanted = { + QWebEnginePage::Copy, + QWebEnginePage::ViewSource, + }; + + for (const QWebEnginePage::WebAction which : unwanted) + menu.addAction(page->action(which)); + menu.addSeparator(); + for (const QWebEnginePage::WebAction which : wanted) + menu.addAction(page->action(which)); + + // The guard: the menu really does hold what the assertions below are about, + // so a filter that removed everything, or a page that offered nothing, + // cannot pass by accident. + QCOMPARE(menu.actions().size(), unwanted.size() + wanted.size() + 1); + + MessageView::removeBrowserActions(&menu, page); + + const QList<QAction *> left = menu.actions(); + for (const QWebEnginePage::WebAction which : unwanted) { + QVERIFY2(!left.contains(page->action(which)), + qPrintable(QStringLiteral( + "a browser action survived the filter: %1") + .arg(page->action(which)->text()))); + } + for (const QWebEnginePage::WebAction which : wanted) { + QVERIFY2(left.contains(page->action(which)), + qPrintable(QStringLiteral( + "the filter removed an action the pane needs: " + "%1") + .arg(page->action(which)->text()))); + } + + // No separator left stranded at either edge by the removals, which reads + // as a menu that lost something. + QVERIFY(!left.isEmpty()); + QVERIFY(!left.constFirst()->isSeparator()); + QVERIFY(!left.constLast()->isSeparator()); +} + void TestMessageView::aSearchFromTheDetailsDialogClosesIt() { // The dialog is modal. Without closing it, the query runs and the thread |