Enabling or disabling filters in the filter list doesn't stick, reverts to previous state when the panel is opened again

RESOLVED FIXED in Thunderbird 68.0

Status

defect
--
major
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: jorgk, Assigned: aceman)

Tracking

({regression})

Trunk
Thunderbird 68.0

Thunderbird Tracking Flags

(thunderbird67 fixed, thunderbird68 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

3 months ago

Enabling or disabling filters in the filter list doesn't stick, reverts to previous state when the panel is opened again.

Assignee

Comment 1

3 months ago
Assignee: nobody → acelists
Blocks: 1475817
Severity: normal → major
Status: NEW → ASSIGNED
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Assignee

Comment 2

3 months ago
Posted patch 1538520.patchSplinter Review

The main fix is the change from !this.checked to this.checked as the toggleFilter was storing the inverted value than was displayed.

I changed the listener from 'click' to 'CheckboxStateChange' event, because previously we listened to click and managed (in toggleFilter) what state was displayed for the checkbox. Now when it is a real checkbox element, it shows and manages its state by itself and we only follow and save the state into filter. So the state of the checkbox could change by other means, not just by click (thus onFilterClick wouldn't be called). So let's tie onFilterClick() to the checkbox state better.

Also notice that when you click the checkbox (or to the right of it), it toggles and the filter row loses proper selection. The background highlight turns grey from blue (on Linux). You can use up/down arrows to move the grey bar and then toggling checkboxes using space key produces interesting effects. I do not fix this here, but it is again a regression, it does not happen on TB60. Geoff, please file and fix that separately.

Attachment #9053103 - Flags: review?(geoff)
Reporter

Comment 3

3 months ago

This works OK in TB 66 beta 3, so bug 1475817 can't be the culprit here.

Assignee

Comment 4

3 months ago

Ok, then I guess bug 1455433.

No longer blocks: 1475817
Reporter

Updated

3 months ago
Attachment #9053103 - Flags: approval-comm-beta?
Comment on attachment 9053103 [details] [diff] [review]
1538520.patch

I don't think the change to `CheckboxStateChange` is necessary but it's not worth debating.
Attachment #9053103 - Flags: review?(geoff) → review+
Reporter

Updated

3 months ago
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 68.0
Reporter

Updated

3 months ago
Attachment #9053103 - Flags: approval-comm-beta? → approval-comm-beta+
Assignee

Comment 6

3 months ago

I think otherwise the enabled state of the filter and the checkbox may diverge if e.g. someone sets checkbox.checked = value instead of clicking it.
So let's try the patch as is, thanks.

Comment 7

3 months ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/9938df594a75
make 'Enabled' filter property in filter list stick by properly checking the state of the checkbox. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.