2074113 - POP move filters become disabled after restart; unresolved destination and wrong-list save for deferred accounts

Status: ASSIGNED

(SeaMonkey :: MailNews: Backend, defect)

Description

[Christoph Bratschi]

Attachment: Review candidate: discover POP move target before disabling; save owning filter list (unbuilt; tests not run)

Environment: SeaMonkey 2.53.24 on Windows, build 20260719163442. Still occurring as of 2026-09-21.

Follow-up to https://bugzilla.mozilla.org/show_bug.cgi?id=594746 and https://bugzilla.mozilla.org/show_bug.cgi?id=618527. The original report is RESOLVED INCOMPLETE, and my account cannot comment or attach a patch there, so I am filing this linked report with current evidence and a review candidate.

I previously reported this in bug 594746 comments 9, 15 and 17, and in bug 618527. Some move filters become unchecked after restarting, leaving matching messages in the inbox; manually enabling them again restores operation. This remains disruptive after many years.

Source review now identifies a concrete internal mutation and a separate persistence problem. My earlier suggestion of an exit-code race was a hypothesis; it has not been established.

Observed profile facts (read-only inspection):

• 381 move filters in the main POP account; all 381 were enabled in msgFilterRules.dat at inspection, whose modification time preceded the current process startup.
• 366 rules target Local Folders; every corresponding decoded mailbox path exists on disk. This does not prove runtime folder initialization or writability.
• Other POP accounts defer to that main account.
• The filter log contained no failure history. No debugger trace or controlled restart reproduction was captured, so the precise triggering branch is not yet proven.

Source findings against tag 2_53_24_final:

https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/blob/2_53_24_final/mailnews/local/src/nsParseMailbox.cpp

1. ApplyFilterHit resolves a move destination through RDF GetResource (around lines 2062–2070). MoveIncorporatedMessage (around lines 2535–2553) treats a null parent or !canFileMessages as an invalid destination and explicitly calls filter->SetEnabled(false).
2. GetParent only returns the existing parent reference; folder discovery is lazy. This is consistent with an existing but undiscovered folder being treated as invalid, particularly during background retrieval before the mail UI initializes the hierarchy. That connection is a source-based hypothesis, not a captured reproduction.
3. ApplyFilters can execute m_deferredToServerFilterList through the same callback, but the disable branch always saves m_filterList. When the disabled rule belongs to the deferred-to account, the wrong list is saved. This can explain disabled in-memory state while the owning file still records enabled rules.

Supporting source:

• https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/blob/2_53_24_final/mailnews/base/util/nsMsgDBFolder.cpp#L3110
• https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/blob/2_53_24_final/mailnews/local/src/nsLocalMailFolder.cpp#L212
• https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/blob/2_53_24_final/suite/mailnews/content/FilterListDialog.js#L571

A candidate patch is prepared for review:

• When the destination has no parent, search its server's real folder hierarchy using GetChildWithURI before deciding it is invalid.
• Return lookup/property failures without disabling the rule. Retain existing behavior for a genuinely missing/non-filing destination after successful discovery.
• Save filter->GetFilterList(), the rule's owning list, when intentionally disabling it.
• Add three candidate xpcshell cases: cold nested destination with the source account's rule; the same with the deferred-to account's rule; genuinely missing destination with owning-file reload, an unrelated-source-file sentinel, and inbox-message retention checks.

Validation: the patch applies and reverses cleanly against the release-tag files; the test JavaScript passes a syntax check. The C++ has NOT been compiled and the xpcshell cases have NOT been executed. No patched binary was installed. This is a review candidate, not a verified fix. Maintainer CI should demonstrate the new tests fail on the baseline and pass with the patch, and run the existing POP3 move/filter tests for both storage backends.

The candidate is deliberately limited to this automatic POP move path. The analogous manual-filter disable branch and other possible initialization failures need separate evaluation.

Observed workflow (intermittent; no deterministic reproduction captured): enable the move filters, restart SeaMonkey, retrieve mail, then inspect filter checkboxes and the inbox. Some matching messages remain in the inbox and affected filters are unchecked. Expected: enabled filters remain enabled and existing destinations are resolved before a move is rejected. The attached cold-destination test is a proposed controlled reproducer, not a test I have successfully run.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: WIP-2074113-bug-594746-candidate.patch

Sorry for the late reply. Long week. The patch looks formally 99% ok but I don't have a pop account any longer to test. I formatted the patch and moved the variable discoveredFolder into the block where it was needed.
I assume it may have been generated with AI help and not tested. I did a quick build and you can find a test build with the patch here:
https://seafiles.thereisonlyxul.org/nightly/2026/09/2026-09-24-00-00-00-comm-253/

If you want another lnaguge just holler. Please check if it does what you intend to accomplish first. If yes please ask for review again.

Comment 2

[Christoph Bratschi]

Comment on attachment 9648242 [details] [diff] [review]
WIP-2074113-bug-594746-candidate.patch

Thank you for the build and cleanup. Yes, the original candidate was AI-assisted, and its C++ and proposed xpcshell tests were not run before submission, as disclosed in the report and attachment.

On 2026-10-18 we tested your English x64 build (2.53.25, BuildID 20260924225145) against 2.53.24 (BuildID 20260719163442) inside a Windows 10 VirtualBox guest, using the same standalone chrome harness, fresh synthetic mbox profiles and a loopback-only POP server. No real mail accounts or user profile were used. This was not a run of the upstream xpcshell suite.

The full GUI application eagerly discovered the destination during account registration, so the final test explicitly resets the synthetic destination's folder cache with nsIMsgFolder.Shutdown(true), then asserts that the RDF destination has a null parent before retrieval. This is controlled coverage of the proposed error state, not proof of the spontaneous startup trigger reported originally.

Results:

1. Source-owned move rule to an existing nested but unresolved folder: 2.53.24 disables the rule, retains the message in the inbox and persists the disabled state. Your build discovers the destination, moves the message and keeps the rule enabled on disk and after a clean restart.
2. Receiving account's rule applied during deferred POP retrieval: 2.53.24 disables the rule in memory and leaves the message in the inbox, while the owner file remains enabled. Your build moves the message and preserves enabled state and mailbox contents across restart.
3. Genuinely missing destination: both disable the rule in memory and retain the message in the inbox. The old build saves the unrelated source list, including an intentionally unsaved sentinel name, while the actual owner file stays enabled. Your build persists the disabled rule in its owning list, leaves the unrelated saved name unchanged and does not create the missing target. These results also persist after restart.

Your build passed all 27 assertions; 2.53.24 passed 11 and failed 16 of those same assertions. All twelve final retrieval/restart application invocations exited normally, and all six POP sessions retrieved the synthetic message and ended with QUIT. The unpacked test-build core required the Microsoft CRT; we supplied valid Microsoft-signed runtime DLLs app-locally in the disposable copy, without changing guest system DLLs.

The tested behavior matches the intended fix. We have not yet tested the real profile, the original natural startup timing, maildir/IMAP/manual-filter paths, non-filing targets or discovery failures. The proposed xpcshell tests remain unexecuted. Please proceed with review of the WIP patch on the basis of this scoped runtime comparison.