Bugzilla

This is a refreshed patch (almost all hunks didn't apply). Addressed the review comments from Magnus and Ian. Updated to current syntax and the new .jsm file even passes eslint.
I fixed the logic problem with running all filters, now it runs only enabled filters of the Periodic type. Another problem was that nsIMsgIncomingServer.setIntValue (getIntValue) only handle int32 values, thus storing Date.now() overflowed the value, causing the nextFilterTime always be in the past, thus filters were run at each check (1-minute timer), instead of the 10 minutes set in mail.server.default.periodicFilterRateSeconds . So the '/ 1000' in the new patch is to store seconds instead of milliseconds, which do fit into int32. It will overflow in year 2038. If that is a problem we can make it / 10000, thus counting in 10-second granularity which would still be enough for this use case (we do not want the filters run too often and with seconds precision).

Another change I made is I stripped the folder flag to set which folders the periodic filters apply to, as this is subject to further discussion and possibly different implementation. I hard-coded that the periodic filters only run on Inbox and it is clearly displayed to the user. In this way it does not yet cover all user use-cases, but one clearly communicated one, where the user can choose if he wants this or not without surprises.

I suggest we could land this confined to Inbox to also see the perf impact for users actually enabling the periodic filters.
I can look at per-filter selector of folders to apply to in a followup bug.

Comments welcome :)

This is a refreshed patch (almost all hunks didn't apply). Addressed the review comments from Magnus and Ian. Updated to current syntax and the new .jsm file even passes eslint.
I fixed the logic problem with running all filters, now it runs only enabled filters of the Periodic type. Another problem was that nsIMsgIncomingServer.setIntValue (getIntValue) only handle int32 values, thus storing Date.now() overflowed the value, causing the nextFilterTime always be in the past, thus filters were run at each check (1-minute timer), instead of the 10 minutes set in mail.server.default.periodicFilterRateSeconds . So the '/ 1000' in the new patch is to store seconds instead of milliseconds, which do fit into int32. It will overflow in year 2038. If that is a problem we can make it / 10000, thus counting in 10-second granularity which would still be enough for this use case (we do not want the filters run too often and with seconds precision). We could even store minutes (/ 60000) because the check still happens in one minute intervals so setting e.g. 90 seconds will not be honored as precisely. We could then switch the pref to also contain minutes.

Another change I made is I stripped the folder flag to set which folders the periodic filters apply to, as this is subject to further discussion and possibly different implementation. I hard-coded that the periodic filters only run on Inbox and it is clearly displayed to the user. In this way it does not yet cover all user use-cases, but one clearly communicated one, where the user can choose if he wants this or not without surprises.

I suggest we could land this confined to Inbox to also see the perf impact for users actually enabling the periodic filters.
I can look at per-filter selector of folders to apply to in a followup bug.

Comments welcome :)