Skip to content

fix(filters): use correct Thunderbird enum and union values - #175

Open
rdkr wants to merge 2 commits into
TKasperczyk:mainfrom
rdkr:fix-filters
Open

fix(filters): use correct Thunderbird enum and union values#175
rdkr wants to merge 2 commits into
TKasperczyk:mainfrom
rdkr:fix-filters

Conversation

@rdkr

@rdkr rdkr commented Jul 23, 2026

Copy link
Copy Markdown

Hi, I started using this project to try to make some filters but ran into an issue with copyToFolder not working. I had Claude look into it and it found the enum mismatch issue; from there I had it verify all of the other ENUM values. During this process it also found the issue addressed by the second commit. I had it update the research docs in a way I think makes sense to avoid confusing anyone - especially agents - in the future. I then had it run some manual tests using the built addon and MCP server. Below is Claude's summary. Thanks!


Summary

Filter creation was broken for most non-string attributes and several actions
because the extension used made-up sequential enum values and always wrote search
values into .str. This corrects both against upstream Thunderbird sources.

Problems fixed

1. Wrong enum values for attributes and actions

ATTRIB_MAP and ACTION_MAP were numbered sequentially, but nsMsgSearchAttrib /
nsMsgRuleActionType are not sequential past the first few keys. So values were
wrong from ageInDays / copyToFolder onward.

  • Most visibly, copyToFolder mapped to 2 (ChangePriority), so every copy
    action threw NS_ERROR_ILLEGAL_VALUE
    .
  • label (8) no longer exists upstream and was dropped.
  • deleteBody (18) was actually KillSubthread and is renamed.
  • custom is -1.

OP_MAP was already correct and is unchanged.

2. Wrong nsIMsgSearchValue union member

nsIMsgSearchValue is a tagged union — date, ageInDays, size, priority,
status, hasAttachment and junk values each live in their own member, not in
.str. buildTerms wrote .str for every attribute, so creating a non-string
filter condition threw NS_ERROR_ILLEGAL_VALUE; serializeFilter only read
date; and the updateFilter copy path preserved only .str and .date.

New setSearchValue / readSearchValue helpers dispatch on the attribute to the
correct union member (via a VALUE_KIND map), and buildTerms, serializeFilter
and the updateFilter copy path all route through them. Malformed input now throws
a clear error at create time rather than persisting a broken filter.

HasAttachmentStatus (44) is handled as a status attribute: it stores the fixed
nsMsgMessageFlags.Attachment (0x10000000) flag in .status, with has/hasn't
decided by the operator and the value token ignored — matching Thunderbird's own
behavior.

References

All members and enum values match upstream nsMsgSearchCore.idl,
nsMsgFilterCore.idl, nsIMsgFilter.idl, nsMsgSearchValue.cpp and
nsMsgSearchTerm.cpp @ 72b8ba0.

Docs

docs/filter-api-research.md Section 5 implementation sketches are replaced with
pointers to the canonical handlers, and Section 6 gets an exact union-member table.

Verification

Manual end-to-end tests of 7e5bc32 (enum values) and 3ac7b01 (union members),
run against live Thunderbird (IMAP) via the MCP filter tools:
create → listFilters readback → assert → delete. All passed. The two existing
user filters were left byte-identical to baseline; all throwaway filters cleaned up.

Tests

  • Conditions (all attributes): subject, date, ageInDays, size, priority, status,
    junkStatus, junkPercent, hasAttachment (is/isnt), tag, otherHeader — each created
    and read back with matching attrib, operator and value. No NS_ERROR_ILLEGAL_VALUE
    on any non-string condition (the pre-fix failure).
  • Actions (all types): moveToFolder, copyToFolder, changePriority, junkScore,
    addTag, reply, forward, markRead, markUnread, markFlagged, delete, killThread,
    watchThread, stopExecution, killSubthread — each round-trips by name.
  • updateFilter copy path: replacing a filter's actions preserves its existing
    date / ageInDays conditions intact.
  • Error handling: malformed date and non-numeric age each return a clear error
    and persist nothing.

Notes

  • Headline regression fixed: copyToFolder (previously mapped to ChangePriority and
    threw) now creates and reads back correctly.
  • Date values round-trip exactly (e.g. 2026-01-012026-01-01T00:00:00.000Z).
  • hasAttachment stores the fixed attachment flag and ignores the value token;
    has/hasn't is carried by the operator — matching Thunderbird.
  • POP-only actions (deleteFromServer, leaveOnServer, fetchBody) and custom
    can't be created on an IMAP account, so their enum mappings were verified by source
    inspection rather than a live round-trip.

rdkr and others added 2 commits July 23, 2026 12:26
…utes

ATTRIB_MAP and ACTION_MAP were numbered sequentially, but the underlying
nsMsgSearchAttrib / nsMsgRuleActionType enums are not sequential past the
first few keys, so values were wrong from ageInDays / copyToFolder onward.
Most visibly, copyToFolder mapped to 2 (ChangePriority), so every copy
action threw NS_ERROR_ILLEGAL_VALUE.

Also: label (8) no longer exists upstream and was dropped; deleteBody (18)
was actually KillSubthread and is renamed; custom is -1. The hardcoded
enum comparisons in serializeFilter / buildActions were updated to match.

OP_MAP was already correct and is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nsIMsgSearchValue is a tagged union: date, ageInDays, size, priority, status,
hasAttachment and junk values each live in their own member, not in .str.
buildTerms wrote .str for every attribute, so creating a non-string filter
condition threw NS_ERROR_ILLEGAL_VALUE; serializeFilter only read date; and the
updateFilter copy path preserved only .str and .date.

setSearchValue/readSearchValue dispatch on the attribute to the correct union
member (VALUE_KIND map), and buildTerms, serializeFilter and the updateFilter
copy path all route through them. Malformed input throws a clear error at create
time rather than persisting a broken filter.

HasAttachmentStatus (44) is a status attribute: it stores the fixed flag
nsMsgMessageFlags.Attachment (0x10000000) in .status, with has/hasn't decided by
the operator and the value token ignored, matching Thunderbird.

Members and enum values match upstream nsMsgSearchCore.idl, nsMsgFilterCore.idl,
nsIMsgFilter.idl, nsMsgSearchValue.cpp and nsMsgSearchTerm.cpp @ 72b8ba0.

docs: replace the Section 5 implementation sketches with pointers to the
canonical handlers, and give Section 6 an exact union-member table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ncrosty58 pushed a commit to ncrosty58/thunderbird-mcp that referenced this pull request Aug 9, 2026
A HasAttachmentStatus term always stores nsMsgMessageFlags.Attachment
(0x10000000) in .status; has-vs-hasn't is carried by the operator (is/isnt),
not the value. Writing a caller-supplied number produced a filter that
silently matched nothing.

Credit to rdkr, who identified this in PR TKasperczyk#175.

Verified against Thunderbird 140.13.0esr: a hasAttachment condition now
writes "(has attachment status,is,true)" to msgFilterRules.dat.
@ncrosty58

Copy link
Copy Markdown

I hit the same enum/union bugs independently and opened #192 before spotting this PR — apologies for the duplicate. Yours is first and I'm closing mine. Two things from my branch that might be worth folding in here.

1. Date conditions are written a day early west of UTC

setSearchValue uses Date.parse(raw) for the Date case. A date-only string like 2024-01-01 is parsed as UTC midnight, but Thunderbird renders and stores filter dates in local time, so the stored day shifts back by one anywhere west of UTC.

Reproduced on this branch (America/New_York), creating a filter with {attrib: "date", op: "isBefore", value: "2024-01-01"}:

condition="AND (date,is before,31-Dec-2023)"

Both code paths side by side:

TZ: America/New_York
input           : 2024-01-01
this branch     : 31-Dec-2023   <-- wrong day
with fix below  : 01-Jan-2024

This is easy to miss because it is silently off by one rather than an error, and it inverts the boundary the user asked for — an "archive everything before 2024" filter leaves 31-Dec-2023 behind.

The fix is to treat a date-only string as a local calendar day and leave anything carrying explicit timezone information to Date.parse:

function parseFilterDate(raw) {
  if (typeof raw === "string") {
    const m = /^(\d{4})-(\d{2})-(\d{2})$/.exec(raw.trim());
    if (m) {
      // Date-only means a local calendar day. Date.parse() reads it as UTC
      // midnight, which Thunderbird then renders in local time, shifting the
      // stored day by one west of UTC.
      return new Date(Number(m[1]), Number(m[2]) - 1, Number(m[3])).getTime();
    }
  }
  return Date.parse(raw);
}

Then const ms = parseFilterDate(raw); in the "date" case.

2. Tests that pin the constants to the IDL

My branch adds test/filter-enum-values.test.cjs, which parses ATTRIB_MAP / ACTION_MAP out of api.js rather than restating them, so the values cannot drift from upstream without failing. It includes explicit regression guards for the two worst cases — stopExecution colliding with DeleteFromPop3Server, and the UTC date shift.

It is written against my naming (NUMERIC_VALUE_FIELDS), so it would need light adaptation to your VALUE_KIND switch. Happy to open it as a follow-up against this branch once it merges, or you're welcome to lift it:

https://github.com/ncrosty58/thunderbird-mcp/blob/fix-filter-enum-values/test/filter-enum-values.test.cjs

Also

I diffed the two branches: the enum maps agree exactly — 17 shared ATTRIB_MAP keys and 18 shared ACTION_MAP keys, zero value conflicts. You additionally have custom: -1, I additionally had anyText: 15.

Your hasAttachment handling is more correct than mine was — I was writing the caller's raw value into .status, which yields a filter that matches nothing. I've taken MSG_FLAG_ATTACHMENT from this PR with credit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants