diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-19 10:46:45 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-19 10:46:45 +0200 |
| commit | a36fff5617f16ac1d17c0c52f2112a20c4fa9336 (patch) | |
| tree | 7da72c963d071fe1563a4b4d9912b03dfad6d21a /src | |
| parent | 98918f4c5f4e05aef1309de823e78da1f50c732d (diff) | |
| download | qtmaildir-a36fff5617f16ac1d17c0c52f2112a20c4fa9336.tar.gz qtmaildir-a36fff5617f16ac1d17c0c52f2112a20c4fa9336.zip | |
feat(pane): offer Select all, and report what a copy copied
Items 115 and 117, both from the user's notes.
Select all was never in Chromium's menu for this pane, measured by hand with a
selection active and against a build with removeBrowserActions() reverted, so
the filter is not what removed it. MessageView::addPaneActions() supplies it,
static and taking the menu, mirroring removeBrowserActions() beside it. Two
comments claiming the standard menu already offered it are corrected; either
would have sent the next reader down the same three wrong theories the item
records.
The copy entries all worked and none of them said so. Four now report through
the pane's existing statusMessage, each naming what it copied rather than saying
"Copied", which is the item's own constraint when three of them sit together in
one menu. Connected to the page's own QActions, so the report follows the entry
wherever it is triggered from.
The two differ in what can be tested, and the tests say so rather than papering
over it. The copy path is fully covered: triggering the action runs the
production path, and mutations for a duplicated message and an unwired entry
both fail. addPaneActions() is covered, but showBodyContextMenu() CALLING it is
not and cannot be, since createStandardContextMenu() returns nothing outside a
real context-menu event; a mutation deleting that call leaves the suite green,
measured. The call site is a hand test and the test file records that so nobody
adds an assertion that appears to cover it.
The copy strings are QT_TR_NOOP inside an array, which CLAUDE.md warns extracts
nothing at file scope. Verified rather than assumed: lupdate found all four
under the MessageView context, because the array sits inside a member function.
387 finished, 0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
| -rw-r--r-- | src/messageview.cpp | 62 | ||||
| -rw-r--r-- | src/messageview.h | 23 |
2 files changed, 81 insertions, 4 deletions
diff --git a/src/messageview.cpp b/src/messageview.cpp index 68821d8..2dba6d1 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -158,6 +158,38 @@ MessageView::MessageView(QWidget *parent) connect(m_view, &QWidget::customContextMenuRequested, this, &MessageView::showBodyContextMenu); + // Item 115. Chromium's copy entries all work and none of them says so, so + // the pane reports for them. Connected to the page's own QActions, which + // are the same instances the standard context menu holds, so this covers + // the entry wherever it is triggered from and needs no menu of our own. + // + // Each message names WHAT was copied. "Copied" alone is worse than nothing + // when three of these sit together in one menu. + // + // The status bar rather than a floating overlay, at the item's insistence: + // this is where the application already reports transient results and + // where they already expire (item 33). A second mechanism for one job is + // what item 45 recorded when two Sync buttons disagreed. + static const struct { + QWebEnginePage::WebAction action; + const char *message; + } kCopyReports[] = { + { QWebEnginePage::Copy, QT_TR_NOOP("Copied the selected text") }, + { QWebEnginePage::CopyLinkToClipboard, QT_TR_NOOP("Copied the link address") }, + { QWebEnginePage::CopyImageToClipboard, QT_TR_NOOP("Copied the image") }, + { QWebEnginePage::CopyImageUrlToClipboard, QT_TR_NOOP("Copied the image address") }, + }; + + for (const auto &report : kCopyReports) { + QAction *action = m_view->page()->action(report.action); + if (!action) + continue; + const QString message = tr(report.message); + connect(action, &QAction::triggered, this, [this, message]() { + emit statusMessage(message); + }); + } + // Ctrl+wheel zoom. The filter goes on the application rather than on // m_view: the wheel event is delivered to an internal QQuickWidget the // view creates lazily, so there is no child to filter at this point and a @@ -660,10 +692,31 @@ void MessageView::removeBrowserActions(QMenu *menu, QWebEnginePage *page) menu->removeAction(menu->actions().constLast()); } +void MessageView::addPaneActions(QMenu *menu, QWebEnginePage *page) +{ + if (!menu || !page) + return; + + // Item 117. Added explicitly rather than relied upon: Chromium's standard + // menu for this pane does not offer Select all and never did, measured by + // hand with a selection active and against a build with + // removeBrowserActions() reverted. The filter is not what removed it, so + // relaxing the filter would not bring it back. + // + // The action itself already exists and already works; only the entry was + // missing. + if (QAction *selectAll = page->action(QWebEnginePage::SelectAll)) + menu->addAction(selectAll); +} + void MessageView::showBodyContextMenu(const QPoint &pos) { - // The page's own menu first: copy, select all and the rest stay exactly as - // they were. This adds to that menu rather than replacing it. + // The page's own menu first: Copy and the rest stay exactly as they were. + // This adds to that menu rather than replacing it. + // + // "and select all" used to be in that sentence and was wrong: Chromium's + // menu here has never offered it. Item 117 measured that and addPaneActions() + // supplies it below. QMenu *menu = m_view->createStandardContextMenu(); if (!menu) menu = new QMenu(this); @@ -673,6 +726,11 @@ void MessageView::showBodyContextMenu(const QPoint &pos) // apply here. Item 100. removeBrowserActions(menu, m_view->page()); + // ...plus the ones it needs and Chromium does not supply. Item 117. + // Before the search entries, so it sits with Copy rather than after a + // separator at the bottom. + addPaneActions(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 df20e40..0b18769 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -155,8 +155,10 @@ public: /// 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. + /// anyway. Copy and View source are the reason the standard menu is used at + /// all, so the menu is filtered, not rebuilt. Select all is NOT among them: + /// Chromium's menu here has never offered it, which item 117 measured and + /// addPaneActions() supplies. /// /// 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, @@ -170,6 +172,23 @@ public: /// without a rendered document or a shown popup. static void removeBrowserActions(QMenu *menu, QWebEnginePage *page); + /// Adds the entries this pane needs and Chromium's standard menu does not + /// supply: Select all, for now. + /// + /// Item 117. Chromium's menu for this pane has NEVER carried Select all, + /// measured by hand with a selection active and against a build with + /// removeBrowserActions() reverted. Do not assume the standard menu + /// provides it and do not "restore" it by relaxing the filter above, which + /// never removed it. + /// + /// Static and taking the menu for the same reason as removeBrowserActions(): + /// createStandardContextMenu() returns nothing outside a real context-menu + /// event, so the production menu cannot be built in a test at all. A test + /// that hand-builds a QMenu proves what THIS function does and nothing + /// about what Chromium offers, which is the distinction item 117 records + /// after three wrong theories. Keep the two questions separate. + static void addPaneActions(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 |
