User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; de; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; de; rv:1.9.1.7) Gecko/20100111 Lightning/1.0b1 Thunderbird/3.0.1 When the Message Filter window is open from creating a filter, it is not updated to show a filter created via Create Filter from Message. Reproducible: Always Steps to Reproduce: 1. Create a filter, e.g. via Create Filter from Message 2. Window Message Filters opens automatically 3. Create another filter, e.g. via Create Filter from Message Actual Results: Second filter created is not shown in list Message Filters Expected Results: Second filter created is shown in list Message Filters The second filter is shown once on closes and reopens the Message Filters window

I don't know if this is TB-specific or not, but to find it in the future it needs to be in Mailnews Core/Filters

I can confirm this is still in TB10, Win XP. If the Filters dialog is minimized, then it is restored and focused after the filter is created, but it doesn't contain the new filter.

I get this in the Error console: Error: TypeError: desiredWindow.refresh is not a function Source File: chrome://messenger/content/mailWindowOverlay.js Line: 3145 Error: An error occurred executing the cmd_createFilterFromMenu command: TypeError: desiredWindow.refresh is not a function Source File: chrome://global/content/globalOverlay.js Line: 95 Depends on which Create filter from menu item I use (there is also one in the context menu of From and Subject in a message view)

This seems to do it. There is infrastructure to open/refresh the filter list dialog after a new filter is created. But the filter list had no function of "refresh" defined so it silently failed (only with error in Error console). So add the function and do what is needed.

I am not sure if this affects seamonkey too and if they have a equivalent of OpenOrFocusWindow(args, windowType, chromeURL) function that calls window.refresh() on the filter list dialog.

(In reply to :aceman from comment #7) > I am not sure if this affects seamonkey too and if they have a equivalent of > OpenOrFocusWindow(args, windowType, chromeURL) function that calls > window.refresh() on the filter list dialog. http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindowOverlay.js#2996

Thanks, so it seems it does not have anything differing from TB and my solution should work as is. I'll produce a proper patch soon.

Comment on attachment 649609 [details] [diff] [review] patch v2 Also reset the search box on refresh, in the same way it is must be done in onNewFilter().

Comment on attachment 649609 [details] [diff] [review] patch v2 Review of attachment 649609 [details] [diff] [review]: ----------------------------------------------------------------- I'm not a TB reviewer and not an official SM MailNews reviewer either. Since the file you're changing lives in TB land (is not shared), you'll need to ask an official TB reviewer. If you choose to adapt the SM version of the file, too, you can ask Karsten (:Mnyromyr) for review of that part. Note that we don't have the filter search box yet. ::: mail/base/content/FilterListDialog.js @@ +107,5 @@ > > +function refresh() > +{ > + // This is called from OpenOrFocusWindow() if the dialog is already open. > + // Just redraw the list. I'd probably put this up as a C-style function comment (like with the next function). @@ +112,5 @@ > + // New filters could have been created by operations outside the dialog. > + rebuildFilterList(gCurrentFilterList); > + > + // As we really don't know what has changed, clear the search box > + // so that the changed/added filters are surely visible. Well, this might come a little surprising to users - after all, they entered something there and you'll clear it without giving them the change to copy it to the clipboard or something.

s/change/chance/

Oh sorry, I overlooked this is TB only. Most of the filter files are in /mailnews so I automatically CC some SM guys... Maybe you need to do same fix in SM version of the file. We clear the searchbox on many ocassions (it is a new feature for several days only) so I just keep consistency. Surely we need to think about optimizing this resetting, but not in this bug.

(In reply to :aceman from comment #14) > Oh sorry, I overlooked this is TB only. Neither the issue nor this bug is, only your proposed fix (no offense!). Basically everything under mail/ is TB-only, everything under suite/ is SM-only, and the rest of c-c has a high probability to be shared. > Most of the filter files are in > /mailnews so I automatically CC some SM guys... Much appreciated. > Maybe you need to do same fix in SM version of the file. Well, eventually someone should, yes, and it might be me. Or it could be you, right here. :-)

I can't do it as SM has a completely different implementation of the list so I don't know what your 'rebuild' command is. Please do the SM version.

Comment on attachment 649609 [details] [diff] [review] patch v2 So I tried this briefly, and it seems to work, so I am fine with it. The question I had though is the interaction with attempts to improve the performance of the filter list dialog. The more important problem is to make sure that we have a usable interface for people with lots of filters, which frankly we have not had ever since the de-rdf work that changed all of this. Does it make it significantly harder to support faster editing of the filter list dialog if we have to maintain the ability to add a filter separately while it is open? I would rather WONTFIX this bug (perhaps just overwriting the filter list with the list in the dialog, losing the added message filter) since this is an edge case, if it makes it more difficult to support rapid editing of a long filter list. But I could not generate a failure in my brief tests, so perhaps this is not an issue.

Comment on attachment 649609 [details] [diff] [review] patch v2 Much better. Thanks! ui-r=me.

(In reply to Kent James (:rkent) from comment #17) > The question I had though is the interaction with attempts to improve the > performance of the filter list dialog. The more important problem is to make > sure that we have a usable interface for people with lots of filters, which > frankly we have not had ever since the de-rdf work that changed all of this. > > Does it make it significantly harder to support faster editing of the filter > list dialog if we have to maintain the ability to add a filter separately > while it is open? > I would rather WONTFIX this bug (perhaps just overwriting > the filter list with the list in the dialog, losing the added message > filter) since this is an edge case, if it makes it more difficult to support > rapid editing of a long filter list. I have not seen any bugs/proposals on how to speed the dialog up, except my bug 780473. So with the absence of any proposals, I can't answer that and I do not see any problem implementing the bug here. Having the refresh function and comments in the code will make future optimizers be aware of the out of dialog filter additions and they will have to think about it.

This depends on bug 780473 as that changes the arguments to rebuildFilterList.

Comment on attachment 649609 [details] [diff] [review] patch v2 works fine for me. please land if you can! thanks :)

Congrats to Axel, for getting the proper bugzilla rights to give f+:) Anyway, I'd wait with this until bug 783110 is landed, from which I'll use the new searchbox resetting function. Even when we reset the searchbox here undonditionally.

So this is an uptodate version. And we need to reset search before list refresh in the new flow :)

:InvisibleSmiley, as there is no rebuildFilterList() is the SM version I have no idea what to do there. You'll need to produce the SM version yourself.

Comment on attachment 662608 [details] [diff] [review] patch v3 Review of attachment 662608 [details] [diff] [review]: ----------------------------------------------------------------- Please use 2 space indention for new code (though yes the file is a bit mixed style). Other than that, looks good! r=mkmelin

Thanks.