Boolean Filter Rules: Adding 2nd and 3rd level groups can cause open brackets (and thereby move 1st level rules to the 2nd level)

[ThiloteE]

Follow up to #35 and #60.

I did some tests using the linux build you sent me here (91.10.0-bb33 (64-bit) and stumbled upon this.

How to reproduce:

1. 1st level match ALL + 2nd level match ANY + 3rd level match ALL:

2022-06-10.16.20.44.2.ALL.3.ANY.for-github.mp4

2. 1st level match ALL + 2nd level match ALL + 3rd level match ANY:

2022-06-10.16.55.27.2.ANY.3.ALL.-.for.github.mp4

[ThiloteE]

A specific requirement for triggering the misbehavior seems to be adding the 3rd level group (I have not tested for 4th level and more levels), as the following two configurations now seem to be fixed and work well:

[Betterbird]

Thanks for the videos. Maybe in the future you can just report the bug with two images while making the window big enough to fit everything.

is incorrectly restored as

We can inspect the content of the msgFilterRules.dat file ourselves, or just paste the content here.

[Betterbird]

We can reproduce the issue. The msgFilterRules.dat has the correct content:

condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c) AND (subject,contains,d] AND (subject,contains,e)"

The bracket at "d" closes two groups, that's by design. Remember that the condition tree is flattened into a list of terms and the terms carry a begin/end group. And obviously the flag can only be set once, even if two groups end on the term. In the debug it looks like this:

entry 0: and   level 1"
entry 1: and begin  level 2"
entry 2: and begin end level 3"
entry 3: and   level 1"

Sadly when this is restored, the "e" condition shifts from level 1 to level 2 since the double-end is not taken into account. The rule execution should still work as long as you don't edit the group again. This is a tricky one.

[Betterbird]

We've detected another problem. If you construct

you get

condition="AND (subject,contains,a) AND [subject,contains,b] AND [subject,contains,c) AND (subject,contains,d) AND (subject,contains,e]"

which is missing a ] at "d" and the ]at "b" isn't right. This seems to depend on the creation order since we also saw:

condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c) AND (subject,contains,d) AND (subject,contains,e]"

what only the term for "d" isn't right.

[Betterbird]

Looks like there is also an issue with something like:

That's [[[a] && b] && c] && d
Of course we can't store three "begin groups" in term "a". Looks like we'll have to change the data structure of the term to add storing how many groups begin/end there.

[Betterbird]

We're working on it. The design will be changed to balance the brackets, so

condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c) AND (subject,contains,d]] AND (subject,contains,e)"

Note the double bracket after "d". Expect a fix in the news few days.

[Betterbird]

The following cases work now with the latest commit:

name="Ex3"
enabled="yes"
type="17"
action="Mark flagged"
condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c) AND [subject,contains,d]]]"
name="Ex1"
enabled="yes"
type="17"
action="Mark flagged"
condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c) AND (subject,contains,d]] AND (subject,contains,e)"
name="Ex2"
enabled="yes"
type="17"
action="Mark flagged"
condition="AND (subject,contains,a) AND [subject,contains,b) AND [subject,contains,c] AND (subject,contains,d] AND (subject,contains,e)"

The case from #62 (comment) doesn't work yet.

[Betterbird]

@RealRaven2000: Please note that significant changes are being implemented here. Most prominently, we're changing nsIMsgSearchTerm.beginsGrouping and nsIMsgSearchTerm.endsGrouping from bool to unsigned long so we can store the number of groups starting and ending at a given term. You will remember the discussion we had privately about multiple groups terminating at a term leading to an [ ] imbalance. Now it's always balanced. We don't know whether your add-ons create search terms. If so, they'd need to be adjusted for work for BB.

[Betterbird]

The latest commit also fixes this case where many groups begin at the same term:

condition="AND [[[[subject,contains,a] AND (subject,contains,b] AND (subject,contains,c] AND (subject,contains,d] AND (subject,contains,e)"

[RealRaven2000]

@RealRaven2000: Please note that significant changes are being implemented here. Most prominently, we're changing nsIMsgSearchTerm.beginsGrouping and nsIMsgSearchTerm.endsGrouping from bool to unsigned long so we can store the number of groups starting and ending at a given term. You will remember the discussion we had privately about multiple groups terminating at a term leading to an [ ] imbalance. Now it's always balanced. We don't know whether your add-ons create search terms. If so, they'd need to be adjusted for work for BB.

Thanks for the heads up. Indeed quickFilters main business is creating filters (so far I don't think I set beginsGrouping during filter generation, but it's definitely important for its backup / restore / copy / paste features). In a nutshell - the idea of the quickFilters assistant is auto-filling thinks like folder target or from address based on emails moved / tagged; so it needs intimate knowledge of the internals of a filter. Can I use "typeof beginsGrouping" to determine patch state?

[Betterbird]

Well, if you don't create groups in quickFilters, then you wouldn't be setting {begins|ends}Grouping on the nsIMsgSearchTerm. No idea how you check the type on an attribute defined in an IDL. As discussed before, you can detect BB usage and then set the grouping members to true/false or 1/0 or higher numbers if you created more complex filters. See the condition examples mentioned earlier. Attached also a sample filter file what will work once an updated version of BB (91/102 beta) is released later today.
msgFilterRules.dat.txt

[RealRaven2000]

Well, if you don't create groups in quickFilters, then you wouldn't be setting {begins|ends}Grouping on the nsIMsgSearchTerm. No idea how you check the type on an attribute defined in an IDL. As discussed before, you can detect BB usage and then set the grouping members to true/false or 1/0 or higher numbers if you created more complex filters. See the condition examples mentioned earlier. Attached also a sample filter file what will work once an updated version of BB (91/102 beta) is released later today.

Thanks for the example! I will find a way to do this - even though my current filter templates don't create groups, users are allowed to generate their own templates and thus can add groupings.

Merging new rules into existing filters is going to be challenging, [this adds more search terms to existing filters and will need to detect the correct new position - e.g. at the end of a group with matching search conditions, such as "from contains" with booleanAnd=false - for the merged search terms and move parentheses (endsGrouping) to the last added term]. I will get the new beta as soon as it's available.

[Betterbird]

@ThiloteE: The latest build at https://www.betterbird.eu/downloads/get.php?os=linux&lang=en-US&version=latest should fix the issues you reported and the ones I discovered myself. I discovered more issues: Deleting a term or group out of the middle of a complex expression do not (yet) adjust the counts of beginning/ending groups, so deletion in complex situation will likely cause invalid rules. We filed issue #63 for that.

[ThiloteE]

Thank you :-) I will do some testing eventually. In some days / weeks.

[ThiloteE]

Positive Feedback:

I have now used BB 91.10 for ~ 6-7 weeks with some "simple" complex filter rules on my main machine and have not noticed any other bugs (apart from #63) so far. Hooray :)

Today I have finally made the jump to BB 91.12.0
I went through all my filterrules.dat files manually (via text editor) and changed the rules to adhere to the new filterrules syntax. 👍

[Betterbird]

Thanks for the feedback.