QuickFilterManager.killFilter always removes the last filter

RESOLVED FIXED in Thunderbird 5.0b1

Status

Thunderbird
Search
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: opera wang, Assigned: opera wang)

Tracking

Trunk
Thunderbird 5.0b1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Lightning/1.0b2 Thunderbird/3.1.9

QuickFilterManager.killFilter always remove the last one filter

Reproducible: Always

Steps to Reproduce:
1.add two new filters to QuickFilterManager by defineFilter, say filter1,filter2
2.remove filter1 by killFilter(filter1)
3.
Actual Results:  
filter2 was removed

Expected Results:  
filter1 was removed

typo?

current code is:
  killFilter: function MFM_killFilter(aName) {
    let filterDef = this.filterDefsByName[aName];
    this.filterDefs.splice(this.filterDefs.indexOf(aName), 1); // ERROR
    delete this.filterDefsByName[aName];
  },

The ERROR line should be:
this.filterDefs.splice(this.filterDefs.indexOf(filterDef), 1);

note that aName changed to filterDef
(Assignee)

Comment 1

7 years ago
see both in 3.3a3 and 3.1.9
Version: unspecified → Trunk
(Assignee)

Comment 2

7 years ago
Created attachment 521133 [details] [diff] [review]
fix the bug in killFilter

patch just for the code, don't know how to add test cases.
Assignee: nobody → opera.wang
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #521133 - Flags: review?(bugmail)

Updated

7 years ago
Summary: bug in QuickFilterManager.killFilter → QuickFilterManager.killFilter always removes the last filter
Comment on attachment 521133 [details] [diff] [review]
fix the bug in killFilter

Oops; that was a pretty dumb bug.  Thanks very much for the patch!  Because this is not a user-facing bug, the bug was typo-class, the specific bug is unlikely to regress, and we have no unit tests for the extension API for quick filters, I'm going to say we don't need a test for this.

The tree is currently closed, but we'll land this on comm-central for release in 3.3.  Because this is an extension API and extensions targeted at 3.1 probably already have to work around this anyways, I don't think it makes sense to land it in 3.1.
Attachment #521133 - Flags: review?(bugmail) → review+
Keywords: checkin-needed
(Assignee)

Comment 4

7 years ago
Yes, I'll make work around in my extension (Expression Search) and later after I re-org it, I needn't to call killFilter.
Checked in: http://hg.mozilla.org/comm-central/rev/d96b3ce7dbed
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
You need to log in before you can comment on or make changes to this bug.