68505 - Can't delete mail filter!

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P1)

Description

[Håkan Waara]

I can't delete any mail filter in the "Message Filters" dialog. I tried to hit
the "Delete" button and pressing delete and backspace. So did timeless, and he
couldn't remove a filter either in win2k..

This is a major loss of functionality.

Comment 1

[timeless]

JavaScript error:
chrome://messenger/content/FilterListDialog.js line 180: getBundle is not 
defined

Comment 2

[laurel]

Still exists in all platforms with feb13 commercial trunk build

Comment 3

[Mark Olson]

This should have been fixed in today's builds, based on timeless' checkin for my
fix to 61801.  (It certainly works here).  I'll take a look at this.

Comment 4

[laurel]

FYI -- still happens with feb14 commercial trunk.

Comment 5

[gayatrib]

I have a patch ready to fix this. Will check it in soon.

Comment 6

[Mark Olson]

This is odd.  My changes in 61801 fixed this for me (the problem was that the
confirm dialog couldn't get the prompt string).  It still works fine here
updated to the tip.  Is there something I'm overlooking?

Comment 7

[scottputterman]

marking nsbeta1+ and moving to mozilla0.9.  This still happens to me on
2001021505 on NT.

Comment 8

[gayatrib]

I had introduced this bug--when I removed the string bundle for getting search 
properties from the FilterListDialog.js -- thinking it was not required. It was 
my mistake.

Attaching a patch--reintroducing getting string bundle for search into 
FilterListDialog.js. With this, delete works. 

Comment 9

[gayatrib]

Attachment: patch

Comment 10

[Mark Olson]

gayatrib, could you take a look at my (somewhat large and tangential) patch in
bug 56680 and see if the equivalent of your changes are already included there?
 If so, I'd like to try to talk everybody into waiting for that (it should
hopefully be fairly soon), rather than me having to merge and rewrite this code.

Comment 11

[gayatrib]

I am not too sure that your patch handles this bug--it does not seem to 
introduce a stringBundle for search properties in FilterListDialog.js.

I feel it would be best to get this patch in. It is just putting back code that 
should have been there--which I took out thinking it was not required.

Adding racham for review and bienvenu for super review.

Comment 12

[Mark Olson]

Actually, my patch uses a xul <stringbundle/> instead of using strres.js to get
at the resource.  It is functionally equivalent to what you are doing in your
patch.  Additionally, my patch gets its string from filter.properties, where the
string currently is, instead of search.properties, where the string used to be.

I'm not trying to be a pain in the butt, but checking this in will unecessarily
complicate my patch, and it won't work with a current tree anyway because of the
moved property.  jag is familiar with my patch.  If he thinks it is more than a
few days off, we should probably take this fix (adjusting it for the correct
properties file of course).

Comment 13

[Peter "jag" Annema]

gayatrib, for what it's worth, the patch in 56680 does address this issue,
though in a different way, like Mark explained in his last comment. After doing
some research it turns out this bug should have been fixed with bug 61801.
Should have been, but wasn't, since not all of the relevant patches were checked
in.

I'll attach an updated version of the patch here, extracted from Mark's patches
in bug 56680. r=jag.

Comment 14

[Peter "jag" Annema]

Attachment: [patch] Fix missing stringbundle, fix strict warnings, clean up a little

Comment 15

[gayatrib]

Tested this. It is working great. Thanks. r=gayatrib.
Seth, could you please sr this?
Can I check in the patch after the sr is done?

Comment 16

[Peter "jag" Annema]

As far as I am concerned, sure. As long as someone checks it in and puts the
correct blame on it :-)

Comment 17

[(not reading, please use [e-mail] instead)]

sr=sspitzer

Comment 18

[gayatrib]

Patch checked in. Marking fixed.

Comment 19

[Peter "jag" Annema]

Where the correct blame would have been mao (Mark Olsen), not I...

Comment 20

[laurel]

OK using feb26 commercial builds: linux rh6.0, win98 and mac OS 9.0