| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Showing the popup takes focus away from the query bar and the popup
window grabs the keyboard, so keys pressed while it is up are delivered
to the popup. An event filter installed on the line edit therefore never
ran at the one moment it had to, leaving Tab to move focus to the next
widget and Return to reach the thread list and open a message.
Install the key filter on the application instead, which sees events
before any widget receives them. It returns immediately unless our own
popup is visible, so it cannot affect keyboard handling elsewhere. The
FocusIn filter stays on the line edit, where it is correctly scoped: it
only fires with the popup down.
Accepting a completion now also reopens the popup when the caret lands
somewhere more can be offered, so taking "tag:" goes straight on to the
tag list instead of needing a second complete_query. The chain stops on
a stem that is already a complete candidate, which is what every accept
produces. The mouse path chains identically.
The previous tests passed against the broken code because they posted
events straight to the line edit, bypassing the delivery path a real
keypress takes. The new tests route keys through the active popup and
run against a real X display; offscreen does not grab the keyboard and
cannot reproduce this class of bug.
|
|
QLineEdit::setCompleter hands completion to the line edit, which then
resets the completer's completionPrefix to the widget's entire text on
every keystroke. The prefix has to be the stem, so once the query grew
past its first token the whole-line prefix matched no candidate, the
popup stopped appearing, and the two reported symptoms followed: nothing
was there for Tab to accept, and Tab fell through to focus navigation.
Attach the completer with setWidget instead, which keeps the popup
anchored without ceding control of the prefix. complete() dereferences
widget() unconditionally, so leaving it unset segfaults rather than
degrading. Opening the popup then becomes ours to do on every edit.
Extend the existing event filter to route the keys the popup needs while
it is visible, and install it unconditionally now that it does more than
the completion_on_focus case. Enter accepts a completion only while the
popup is up, so returnPressed still runs the query when it is closed.
The existing tests called acceptCompletion() directly and so never
touched the widget, which is why neither bug was caught. Add four tests
that drive the real widget path plus one covering both settings of
completion_on_focus; the first two fail against the old code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Adds the complete_query action, whose registration the assertion in
registerActions() has been demanding since keymap gained the name: a
known action nothing implements would have been a silently dead binding.
Also lands the QueryCompleter half of the manual trigger, which the
keymap commit left behind: triggerCompletion() and the focus-in filter
gated on completion_on_focus.
Tags refresh at startup, after a sync, and after a mutation introduces a
tag not already known, since that is the tag most likely to be typed
again. onAllTagsReady deliberately drops the generation the signal
carries: a tag list is not an ordered query result, so a later one is
always at least as good as an earlier one and discarding on staleness
could only throw away a good list.
|
|
The free-form date hint is a footer label rather than a model row: a row
would be filtered away by the first non-matching keystroke and could be
selected and inserted, producing a query that errors.
QCompleter::setPopup takes a QAbstractItemView, so the label cannot be laid
out beside the view in a container widget. The footer sits in space reserved
with setViewportMargins inside the list view instead.
setItemDelegate must run after setPopup, not before: setPopup installs a
plain QStyledItemDelegate of its own and discards whatever was already set,
which silently drops the description column.
Accepting replaces exactly the span the tokenizer identified rather than
QCompleter's own completion prefix, which is a different span once a prefix
or a range bound is involved. Four tests drive that path directly instead of
through synthetic key events, since whether a key needs Shift is a
keyboard-layout property and could not decide the question.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
tag: and is: share the tag list, since notmuch aliases them. path: offers
each account maildir in both bare and recursive forms, the latter being
what Account::scopedQuery builds and not something a user would guess.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Mimetypes are the one completion list with no enumerator, so the user can
extend it. Entries append to the built-ins and a malformed one is skipped
with a problem recorded rather than dropping the whole list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
KeyMap::defaultBindings() is the single source of truth for shortcuts:
the menus, the shortcut reference dialog and loadDefaults() all read it,
so registering here makes the binding appear in the reference and stay
rebindable from [keys] without any of them disagreeing.
Ctrl+Space is a modifier plus a named key, so it sidesteps the bare
capital trap in normalizeSequence() and needs no Shift on any layout.
Verified it parses to a single non-empty combination that round-trips to
"Ctrl+Space", and it collides with no existing default.
Note test_mainwindow now fails its everyKnownActionIsRegistered()
assertion: MainWindow does not yet implement complete_query. That wiring
is a separate task, and the assertion firing is the intended signal.
|
|
Keywords are syntax and stay untranslated; the descriptions beside them are
prose and go through tr().
The tables are free functions with no QObject to inherit tr() from, so the
file declares a VocabularyStrings context with Q_DECLARE_TR_FUNCTIONS rather
than borrowing QObject::tr, which would file every string under the QObject
context.
|
|
Query bar completion cannot offer tag names without a way to enumerate
them, and libnotmuch had no call wired up for it. Follows the existing
generation-counter pattern; the result crosses the thread boundary as a
QStringList.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Each side of '..' is an independent value against the same model. Entries
that are themselves ranges are withheld once a range exists, since
date:1week....today is malformed.
The token is read to its full extent rather than truncated at the caret:
the separator deciding which bound is being edited can sit to the right
of the caret. Stems stay caret-bounded so matching never uses untyped
text.
|
|
subject:"foo bar has the cursor in free text, where offering keywords
would be wrong.
|
|
The replace span covers the value only, so accepting a completion never
overwrites the prefix that selected it.
|
|
Prefix completion only so far: the token under the cursor, bounded by
whitespace or an opening parenthesis rather than by the start of the line.
|
|
Replace QMessageBox::about with a QDialog laid out in two columns: the
application icon on the left at 40%, the version, description, copyright
and AI-assistance notice on the right at 60%. A full-width row below both
carries the project URL.
The copyright line, GPLv2 notice and AI-assistance disclosure were absent
from the About window despite being present in the source headers and the
README.
|
|
The attachment bar had been an empty placeholder since it was written:
MessageView created it and added it to the layout, and nothing ever put
anything in it. MimeParser had been extracting attachments the whole
time and Attachment::saveTo() already carried the path-traversal guard,
so the backend needed calling rather than writing.
The bar holds one "Attachments (N)..." button whatever the count. One
button per file was built first and was wrong: a thread with sixteen of
them made the bar as wide as the window, pushed the splitter over and
left the thread list a few pixels wide. The button opens a dialog
listing message number, filename and size with a Save each, and a
"Save all..." when there is more than one.
Save all writes into a new subdirectory named "<date> <subject>" inside
a parent the user picks, rather than dropping sixteen files loose among
whatever is already there. Zipping was considered and rejected: Qt ships
no zip API, so a real archive meant a new build dependency or shelling
out to /usr/bin/zip at runtime, and a subdirectory answers the actual
requirement. The picker names the subfolder before the user commits to a
location.
The subject is attacker-controlled and becomes a directory name, so
attachmentFolderName() sits beside the other guards in mimeparser.cpp:
it strips separators, control characters and leading dots, caps the
length, and falls back to a generated name. Its test asserts that every
hostile subject still resolves inside the parent directory.
Two defects surfaced while using it, both silent:
saveTo() overwrites an existing file, and several messages in one thread
commonly attach the same filename. Saving that thread destroyed six of
sixteen files while reporting all sixteen as saved. The batch path now
uses saveWithoutOverwriting(), which appends " (2)" before the extension
and keeps a compound extension whole.
Qt::RFC2822Date rejects a Date header that carries a timezone comment,
"+0200 (CEST)", which is legal per RFC 5322 and common in real mail. Qt
refuses the entire string rather than ignoring the comment, so every
such message lost its date prefix. Comments are stripped before parsing.
Opening an attachment in its default application is deliberately not
included: handing a file from a stranger to xdg-open is a different
security decision from writing it where the user asked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
An attachment was only discoverable by opening the thread. A narrow
leftmost column now marks the threads that carry one.
No new worker query is involved: notmuch applies the "attachment" tag
while indexing, so ThreadSummary already holds what this needs. The
marker is a glyph rather than an icon resource, which ships no new asset
and inherits the row font, so it strikes through with a doomed thread
like every other cell. It falls back to "*" where the system font cannot
draw U+1F4CE, since an unrenderable codepoint reads as breakage rather
than as a marker.
Two silent Qt behaviours had to be handled, both found by probe:
QHeaderView::restoreState() returns true for a blob saved against fewer
columns and applies the old widths shifted one place right. Adding a
column in front would therefore have mangled every existing saved
layout with no error to detect it by. The column count is now stored
beside the blob and a mismatch discards it, so the widths reset once on
upgrade instead of landing on the wrong columns.
QHeaderView's default minimumSectionSize is 58px on this platform, and
setColumnWidth() clamps to it without reporting the smaller value back,
so the column could not be narrow at all until it was lowered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The app opened whichever saved query sorted first alphabetically, which
is not a choice anyone made: [queries] is read through childKeys(), so
savedQueries().first() means "Flagged" before "Inbox" before "Unread"
rather than anything the user expressed.
[general] startup_query names the entry to open and defaults to Unread,
so a fresh install comes up on the unified unread list. Saved-query
button order is untouched and stays alphabetical.
A name matching no saved query falls back to the first one rather than
starting with an empty view. That is reported as a problem only when the
user actually wrote the name; the built-in default naming a query they
never created is not something they got wrong, and warning about it
would fire on every launch of a config that has no Unread entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Zoom was Chromium's, not the application's: the web view handled the
keys natively and never told anyone, so there was no value to save.
qtmaildir now owns it. Zoom in, out and reset are real actions, in the
View menu and rebindable through [keys], and the factor is persisted to
the UI state file. Ctrl+wheel over the body zooms and Ctrl+middle-click
resets, both filtered by ancestry from an application-level filter: the
events are delivered to an internal QQuickWidget the web view creates
lazily, so a filter on the view itself never sees them.
The factor is clamped to 0.5 - 3.0, and NaN, infinity, zero and negative
values fall back to 1.0, since a corrupt state file must not be able to
leave the pane unreadable with no visible way back.
Both risks the plan flagged turned out not to exist, verified by probe
rather than assumed. The application QAction wins over the web view's
native zoom key, so the tracked factor cannot diverge from what is on
screen. And the factor survives setHtml(), so the web view is the single
source of truth and needs no reapply per render.
A third finding is worth recording because it produced a wrong fix
first. A probe using QTest::keyClick() reported Ctrl++ as a dead
binding, and a test was written asserting that. Both were wrong: Ctrl++
is exactly what the '+' key emits on an Italian layout, confirmed
against the real keyboard, and it is the shipped default. Whether a
symbol needs Shift is a property of the layout, not of Qt, and
keyClick() reproduces neither. The test now only checks that every
default parses, and the comment in defaultBindings() says not to
re-derive this from synthetic input.
Ctrl+= is a second binding for reset, skipped when [keys] gives it to
something else.
Also fixes a pre-existing bug the new config key exposed. [general]
entries were read as "general/<key>", which matches nothing: QSettings'
INI backend treats a section literally named [general] as its own
fallback section and strips the prefix. notmuch_config had therefore
never worked. Both keys are now read without it; the file format is
unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Resizing the window, the splitter or a thread-list column was undone by
the next launch. State now round-trips through a separate settings file
at ~/.local/state/qtmaildir/uistate.conf, written on close and read at
startup.
The state file is deliberately not the user's config: a base64 geometry
blob does not belong in a hand-edited file, and rewriting that file on
exit would drop its comments and key order, which QSettings does not
preserve.
Two details that are easy to get wrong:
QStandardPaths::StateLocation appends both the organization and the
application name, and both are "qtmaildir" here, so it resolves to
~/.local/state/qtmaildir/qtmaildir. The path is built from
GenericStateLocation instead, matching Config::defaultPath().
Restore runs after buildMenus() rather than at the end of buildUi():
QMainWindow::restoreState() matches toolbars by object name and silently
drops the position of one that does not exist yet.
Every restore is conditional on a non-empty blob, so a missing or
rejected state file leaves the built-in defaults instead of producing a
zero-size window.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The icon was committed in a previous session and referenced nowhere: no
qrc, no .desktop entry, no setWindowIcon. It is wired up now, as a window
icon, a desktop entry, and install rules placing both into hicolor and
share/applications.
resources.qrc belongs to the executable rather than to qtmaildir_lib. A
qrc compiled into a static library registers itself from a global
initialiser, and the linker discards that object because nothing
references it: the build succeeded, qInitResources_resources() was
present in the .a, and QFile::exists(":/icons/qtmaildir.svg") still
returned false at runtime. Verified loading at 16, 32 and 64 pixels after
the move.
Toolbar and menu actions take icons from the system theme by their
standard names, so they match the rest of the desktop rather than
shipping bespoke art. A theme lacking one leaves that action as text,
which still works.
|
|
Two optional keys in an [account.<key>] stanza. color fills the chip,
label sets its text.
Both belong to the account rather than to [tagcolors] because an account
tag is a different taxonomy: which mailbox a thread arrived in, not what
state it is in.
label is display only. "account-provider-work" is a lot of
row for one bit of information, but the notmuch tag is never renamed, so
existing queries and external tagging are unaffected. Unset falls back to
the account key, and an empty label is ignored rather than rendering a
blank chip.
|
|
Spelled out per row, tags ran to 500 pixels of largely repeated text and
took most of the thread list's width. The column is gone; tags render as
coloured chips split by what they actually mean.
An account tag says which mailbox a thread arrived in, and draws as a
chip in front of the subject. A functional tag says what state a thread
is in, and those fill one row under the message pane. One row keeps the
message area from shifting between threads with different tag counts, so
whatever does not fit collapses into a +N chip that names the rest in its
tooltip.
TagColors resolves a colour by exact tag first, then by top-level prefix,
so a single "shopping" entry covers shopping/amazon and shopping/nike
while shopping/amazon can still override its own. That matters at 96
tags. Built-in defaults cover the usual state tags, and anything left
unconfigured falls back to a hash of the name, stable so a chip does not
change colour as the list scrolls.
|
|
Subject was Stretch, which computes its own width and discards a drag, so
it alone could not be resized. Every column is Interactive now.
Nothing absorbs spare width as a consequence, so the view scrolls
horizontally instead of squeezing columns when their total exceeds the
viewport. Per-pixel, so scrolling does not jump a column at a time.
|
|
Tags, Date and From were set to ResizeToContents while fixing the
pushed-off-screen Tags column. That mode computes the width itself and
discards a drag, so the columns stopped being resizable. Verified: with
ResizeToContents a request for 250px yields 150, with Stretch 478, with
Interactive 250.
The reorder alone already fixed the original bug, since Subject stretches
and is last, so nothing can be pushed past it. Locking the other three
was unnecessary and also blocked the saved-column-widths item, which
needs widths a user can actually set.
They are Interactive again, with starting widths a drag overrides.
|
|
Selecting a thread and hitting Delete changed nothing on screen, so
there was no way to tell the action had stuck.
The tag was always applied: applyTagChange() emitted dataChanged across
the row, and the Tags column did update. But Subject was set to stretch
while Tags came after it, so Subject took all free width and pushed Tags
out of view. The feedback lived in the one column that could not be seen.
Columns are now Tags, Date, From, Subject, with Subject stretching last
so nothing can be pushed off the right edge. A thread tagged deleted or
spam fills its whole row, muted red or orange with white struck-through
text, through the background, foreground and font roles, so no cue
depends on one column remaining visible.
Strike-through accompanies the fill on purpose: it survives a theme that
overrides backgrounds and reads without colour. Bold for unread still
composes with it.
Archive adds no tag, so an archived row is left unstyled for now.
|
|
Fourteen actions in one table made a dialog taller than the display,
which pushed its own title bar off the top. The rows are split into two
columns of seven, with the closing note spanning both.
QMessageBox is replaced by a plain QDialog. The message box wraps its
text at a narrow fixed width, which broke every description into a
column of single words and was most of the height: 719x1084 before,
1426x366 after.
|
|
Actions were a QHash of std::function dispatched by an event filter,
which nothing could put in a menu. They are QActions now, bound from
KeyMap so a [keys] override reaches the menus as well as the keyboard.
Menu bar covers every action; the toolbar carries only Sync, Archive,
Delete and Undo. Help > Keyboard shortcuts is generated from the actions,
so it shows what the keys really do rather than a copy that drifts.
spam and load_remote gained defaults, having been unreachable without a
hand-written binding.
The event filter is gone. Probing showed QAction shortcuts are dispatched
before the focused widget sees the key, so they beat QAbstractItemView's
type-to-search without one, and Qt already suppresses plain-letter
shortcuts while an editable widget has focus. Dropping the filter's
blanket guard also lets Ctrl+Q work while the query bar has focus.
registeredActionNames() is derived from the actions rather than
hand-maintained, so the two drift tests it needed are replaced by checks
that a configured binding reaches its action.
No confirmation dialogs: tag mutations still answer to undo.
|
|
Typing a capital emits Shift+<key>, but QKeySequence::fromString() folds
the case of a bare letter away: "N" parsed to plain Key_N, a combination
no keystroke produces. The N, F and G defaults (toggle_unread, flag and
sync) therefore never fired, and neither would any hand-written capital
in [keys].
normalizeSequence() rewrites a bare capital to Shift+<letter> and is
shared by the defaults and the override pass. As a side effect "y" and
"Y" become distinct keys rather than a collision that dropped one.
Defaults move to modifier shortcuts throughout. A single letter cannot
be a QAction shortcut without stealing that letter from every text field
in the window, and the menus in the next commit need real accelerators.
defaultBindings() is now the one source of truth for them.
|
|
The version was declared in the project() call and used nowhere. It is now
generated into version.h from that single declaration, so nothing repeats
the literal, and it reaches the places it is actually wanted: --version,
--help, the window title, and QApplication.
--version and --help are answered before the web engine schemes are
registered and before QApplication is constructed. Printing one line does
not need a GUI, and both must work on a machine where the GUI cannot open.
Staying at 0.1.0 rather than calling this 1.0.0: under semver, 0.x is where
the interface may still change, and for this project the interface is the
config file format and the bindable action names. Both are one manual
verification pass old. 1.0.0 becomes a deliberate decision to stop changing
them under users.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Confirmed with the maintainer as v2-only rather than v2-or-later. LICENSE is
the official text from gnu.org. Every file under src/ and tests/ carries the
matching notice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
With the thread list focused, 'h' jumped to the next thread whose subject
began with "h" instead of toggling HTML, and j, k, a, d, N, F, u and G were
swallowed the same way. The keymap only worked when focus happened to be
somewhere else.
installEventFilter(this) was on the MainWindow, and a window-level filter
only sees key presses the focused child did not consume. QAbstractItemView
consumes plain letters for its type-to-search feature, so it took them
first. The filter is now installed on the thread view as well, which puts
the keymap ahead of that search. The existing query-bar guard in
eventFilter() still keeps ordinary typing working there.
Found by the maintainer while walking task 13 item 14, and confirmed fixed
on screen.
No regression test: a QTest::keyClick attempt passed both with and without
the fix, because synthetic key posting does not reproduce the focus and
consumption path that causes the bug. A test that cannot fail is worse than
none, so it was dropped rather than kept for appearances.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Granting remote content on one thread, moving to another and coming back
showed the images again with the banner gone. The grant is documented as
never sticky, and it was not: verified against a local HTTP server that the
image is fetched exactly once, under the grant, and never re-requested. The
interceptor's policy was correct throughout and allowRemote was false on
return.
The images came from the engine's decoded-image cache, which is keyed on
the document and consulted before any request exists, so the interceptor is
never asked. Policy right, pane lying.
clearHttpCache() empties the profile's store but not that one. Loading
about:blank first discards the previous document along with its cached
images. This belongs in showThread() rather than render(): render() also
runs for the remote-content grant itself, where throwing the document away
would discard exactly what the user just asked to see.
Found by the task 13 checklist (item 11) and confirmed fixed by the
maintainer on screen.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Clicking a thread left the pane blank. Two independent bugs, both from the
same false premise: that setHtml() navigates to the base URL it is given.
It does not. setHtml() navigates to a data: URL carrying the markup and
applies the base URL afterwards, purely as the document's origin. Verified
empirically on Qt 6.11.
Built on that wrong assumption were:
- MessagePage::acceptNavigationRequest compared the navigation's URL
against documentUrl() and rejected everything else, so the document load
was refused. It now accepts a typed main-frame navigation, which is one
we initiated ourselves.
- RequestInterceptor exempted exactly the qtmaildir: base URL and denied
everything else, so the data: document load was blocked too.
The interceptor fix is scoped to ResourceTypeMainFrame rather than allowing
the data: scheme outright. A blanket allow would have been a real hole: a
message body can write <img src="data:..."> or an iframe, and the existing
dataSchemeBlocked test in test_interceptor.cpp was right to fail when that
was tried. Sub-resource data: URLs remain denied.
Note this was never working. The drafted version had the same defect in a
different spelling (it compared url.scheme() rather than the whole URL, and
would have rejected the data: navigation just the same), and task 11 shipped
with no runtime test to catch it. test_messageview.cpp now pins all three
facts: the document loads, its text reaches the page, and a data: image
inside a hostile body stays blocked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Found while walking the task 13 checklist against real mail. Item 1
("startup shows no configuration warnings with a valid config") failed:
with a perfectly valid config that simply had no [sync] command, every
launch opened a blocking modal that had to be dismissed before the window
could be used.
Config now separates the two cases. A problem is something configured but
wrong (a sync command that does not exist, an account with no maildir);
those still open a dialog, as does every KeyMap warning, since each one
means a binding the user wrote is being ignored. A notice is an optional
feature simply not being configured; it reports to the status bar only.
Nothing is broken in that case, and a modal on every launch teaches the
user to dismiss dialogs unread, which defeats the ones that matter.
problems() is a subset of warnings(), so callers wanting everything need
only the latter.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Wires the worker thread, thread list, message pane, sync process and undo
stack together, and replaces the placeholder main() with real startup:
custom URL schemes registered before QApplication, a libnotmuch ABI check,
and config loading.
Four fixes against the drafted version:
- onWorkerError() only set a status label. Its own comment elsewhere
claimed it reverted the optimistic update, and the spec requires that;
it did not, so a rejected write left the list showing a tag the database
never received. The pending change is now recorded and rolled back, and
a confirmed tagsApplied clears it so a later unrelated error cannot undo
a write that succeeded.
- runCurrentQuery() cleared the model but left the undo stack pointing at
rows that no longer exist. Undoing after a new query would have written
to the database while the visible list stayed put. The stack is cleared
with the model.
- m_currentMessages was assigned on every thread load and never read.
Removed.
- buildUi() connected sync output to m_syncLog and errors to m_statusLabel
before either existed. Both are constructed before the wiring now.
cidPrefix generation lives here, this being its only producer in the
application, and is pinned by tests: it must never contain '!' and must be
distinct per message, which are the invariants the cid: namespacing rests
on. A second test holds registeredActionNames() against
KeyMap::knownActions(), since those two hand-maintained lists drifting
either way silently breaks a user's key binding. Mutation-verified that
dropping an action fails the test by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Off-the-record profile, JavaScript off, deny-by-default interceptor, and a
page subclass that hands link clicks to the system browser so a message can
never navigate the pane.
Honours the obligation task 5 recorded: the interceptor trusts exactly one
qtmaildir: URL and fails closed otherwise, so setHtml() and setDocumentUrl()
must agree or the pane renders nothing. Rather than pairing those calls at
each site, every load goes through one setDocument() and the URL comes from
a single documentUrl() accessor. Verified against the real interceptor that
this URL is allowed while siblings, subpaths, remote and file: are not.
Three fixes against the drafted version:
- showError() called setHtml() with a base URL but never setDocumentUrl(),
so an error card would have rendered blank. Now impossible to repeat.
- clear() and showError() left the previous thread's inline parts in the
scheme handler and its cids in the interceptor. Both now empty the policy,
so no thread's parts outlive it.
- MessagePage trusted the whole qtmaildir: scheme for typed navigations,
which is the same blanket-trust mistake task 5 removed from the
interceptor. It now matches the exact document URL.
The parts-flattening is extracted into buildThreadCidMap() so it can be
tested without a live profile, and a cidPrefix containing '!' is sanitized
rather than trusted, since Q_ASSERT is compiled out in release and this map
decides which bytes a message can name. The sanitizer escapes '_' before
replacing '!', because a plain replace would map "m0!x" and "m0_x" onto one
key and merge two messages, which is the very collision the namespacing
exists to prevent. Mutation-verified: the naive replace fails the
distinctness test, and dropping the sanitizer trips the assert.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Runs the configured sync script through QProcess, merging stdout and
stderr into one log so a failing mbsync run has something to show. The
script is never run through a shell: the command is a config value, and
splitCommand keeps its arguments literal.
Two fixes against the drafted version:
- start() no longer calls waitForStarted(). It blocked the UI thread for
up to five seconds, which contradicts the spec's requirement that the UI
stay usable during sync, and it swallowed launch failures into a bare
false return. A missing script now surfaces asynchronously through
errorOccurred as finished(false, -1) with an explanatory log line, so
the spinner cannot hang with nothing to explain it.
- Removed a double-emit guard I had added on the assumption that QProcess
follows errorOccurred(FailedToStart) with finished(). Verified it does
not: FailedToStart is emitted instead of finished, never before it. The
guard was dead state and the comment justifying it was wrong.
Also corrects the sync interval throughout: the user's cron runs every 10
minutes, not hourly. The shorter interval strengthens the flock rationale
rather than weakening it, since collisions with a manual sync are that
much more likely.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
QAbstractTableModel over query results, appended in batches so a large
query paints its first screenful immediately. Tag changes apply locally
for optimistic UI; reverting a failed write means calling applyTagChange
again with added and removed swapped, which the round-trip test pins.
Two additions to the drafted version:
- A ThreadIdRole, so a view's QModelIndex maps back to the thread id the
worker speaks without every caller reaching around the model.
- data() checks its own row and column bounds. Qt will not hand out an
out-of-range index and invalidates persistent ones on reset, so this is
unreachable defence rather than a live path; the test says so instead of
pretending to cover it.
Verified by mutation that the empty-batch guard, the ThreadIdRole, and the
full-row dataChanged range each fail exactly one test when removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Owns the only notmuch database handle. Queries run read-only and emit
threads in batches of 200 with a generation counter so the UI can discard
superseded results. Tag mutation closes the read-only handle, opens
read-write, applies, and closes, holding the process-wide write lock for
milliseconds rather than blocking a concurrent `notmuch new`.
Tested against a throwaway database built in a QTemporaryDir, superseding
the spec's original "no unit test" position: applyTags is the only code
here that writes to a notmuch index. The fixture never touches ~/Mail or
~/.notmuch-config.
Two fixes against the drafted implementation, both caught by mutating the
code and confirming exactly one test failed:
- loadThread conflated "no query given" with "query matched nothing in
this thread", so filtering a thread down to zero matches rendered every
message expanded. Tracked with an explicit haveMatchSet flag.
- applyTags now documents why a stale message id must skip rather than
abort: notmuch_database_find_message reports SUCCESS with a null message
for an unknown id, and the live ids alongside it still need tagging.
Note for fixture authors: notmuch synchronizes maildir flags with tags at
index time, so a file named `...:2,S` is indexed without the unread tag no
matter what [new] tags requests. Unread fixture messages go in new/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
ThreadSummary, MessageRef, and TagChange are plain-value structs that
carry query results across the worker/UI thread boundary via queued
signals. NmHandle<T, Destroy> wraps libnotmuch's C handles (query,
threads, messages, thread, message, tags) so early returns in the
query paths can't leak.
|
|
The cid: namespacing scheme (cid:<prefix>!<id>) is only unambiguous
because the prefix half is guaranteed free of '!': the first '!' in the
result is always the separator, so an attacker-controlled Content-ID
containing '!' only extends the id half rather than colliding with a
different prefix. That invariant previously existed only as a comment.
Add Q_ASSERT_X at both independent call sites that perform this
concatenation (CidSchemeHandler::namespacedKey and
HtmlBuilder::namespaceCids) so a future prefix generator that violates it
traps in debug builds, per Task 5's precedent of not letting one unit's
correctness depend silently on another's future behaviour. Since
Q_ASSERT compiles out in release, pin the property that actually matters
release builds too test: distinct (prefix, id) pairs across a documented
"m<index>" prefix set and hostile Content-IDs (containing '!', percent-
encoded '!', empty, leading/trailing '!') never collide, and the key
always splits at its first '!' back to the exact original prefix.
|
|
HtmlBuilder renders parsed messages (and whole threads, as one document,
so newsletter threads don't spawn one Chromium process per message) into
the HTML string the web view loads. Plain text is escaped and quote lines
marked; the cid: rewrite is namespaced per message ("<prefix>!<id>") so
two thread messages sharing a Content-ID don't collide.
Hardened namespaceCids beyond the initial sketch after attacking it:
handles unquoted cid: attribute values, background=/poster= (not just
src/href), and CSS url(cid:...) in both style="" attributes and <style>
blocks, all case-insensitively. Replaced the greedy [^"']+ capture with
per-quote-style alternation so two cid: refs on one line can't bleed into
each other.
CidSchemeHandler serves cid: requests from the thread's inline-parts map,
keyed by the same namespaced string, replaced wholesale per thread.
|
|
Whole-scheme allow meant a hostile message body could reference any
qtmaildir: URL (e.g. <img src="qtmaildir://other">) and have it pass,
with safety depending entirely on Task 11's still-unwritten scheme
handler. Add setDocumentUrl() and require an exact QUrl match; deny
all qtmaildir: URLs when it is unset (fail closed). Document URL
survives resetForNewMessage() since it is a property of the view, not
of a message.
|
|
|
|
Attachment::saveTo()'s escape guard compared paths with a bare
QString::startsWith(), which is not a path-boundary test: "/tmp/safe-evil"
textually starts with "/tmp/safe", so a sibling directory whose name merely
extends the target's name would incorrectly pass as contained within it.
Extract the check into Attachment::isPathInsideDirectory(), comparing
QDir::cleanPath()'d absolute paths and requiring an exact match or a prefix
ending at a '/' boundary. Not exploitable today since safeFilename() always
reduces the name to a bare basename before saveTo() builds the target, so
the guard is unreachable via saveTo()'s public interface; comments on both
now say so plainly instead of implying it is currently load-bearing.
Add pathInsideDirectoryRejectsSiblingPrefix, testing the guard directly
(independent of safeFilename(), which would mask a broken guard by never
producing an escaping path), and safeFilenameStripsPathComponents, testing
the sanitiser that actually stops traversal today.
|
|
|
|
Accounts use [account.work] rather than [account/work]: QSettings' INI
backend treats "/" as its own hierarchical group separator, so a literal
slash in a section header parses as a nested group and trips
QSettings::FormatError, silently breaking childGroups() enumeration. A
dot carries no such meaning and keeps the format flat.
Saved-query order is alphabetical (QSettings::childKeys() sorts), not
file order; documented in code and tests rather than left to a false
assumption.
|
|
loadOverrides() inserted straight into m_bindings, so two override
lines that normalize to the same QKeySequence (e.g. "y" and "Y", both
"Y" per QKeySequence) silently overwrote each other with zero warning,
contradicting the "a typo cannot bind silently" contract on
knownActions().
Track sequences seen within the current override pass separately from
m_bindings (which already holds the defaults) so overriding a default
key stays silent, but two colliding override lines produce one warning
naming both actions.
|
|
Maps key sequences to action name strings, with hardcoded vim-style
defaults and QSettings-based [keys] overrides. Unknown actions and
unparseable sequences are collected as warnings rather than treated
as fatal, so a typo in the config cannot silently misbind or crash.
Note: QKeySequence::fromString() on Qt 6.11 does not return an empty
sequence for unparseable input (e.g. "NotAKey++") -- it returns a
non-empty sequence whose toString() is empty. Detection uses that
instead of isEmpty().
|
|
|
