1002006 - MsgFilterList(args) should take targetFilter and pass it to FilterListDialog

Status: RESOLVED FIXED

(Thunderbird :: Filters, enhancement)

Description

[Axel Grude [:realRaven]]

It should be possible to preselect a specific filter by adding

args.filter = targetFilter;

before calling MsgFilterList(args)

It would also be good if we had a mechanism to select a filter when the filter dialog is already open. In my case I have written a merge assistant which opens the filter list after editing the resulting (merged) filter and I would like to highlight this filter within the list. [This functionality is part of my quickFilters addon]

Comment 1

[:aceman]

Attachment: WIP patch

This could do it. Patch to be applied on top of bug 363982.

Comment 2

[:aceman]

Attachment: WIP patch v2

Found a small bug so that the .refresh() way didn't work right when creating new filter in the same account. This version also adds a usage of this functionality in core TB.

The plain for loop over the filters doesn't look too efficient, but I have tested it to select the last filter of 10000 filters and didn't notice any significant slowdown (I mean it got hidden in the slowness of just showing such a filter list:)).

Comment 3

[:aceman]

Attachment: patch v3

Patch updated after changes in bug 363982.

Comment 4

[:aceman]

Attachment: patch v3.1

Axel, now that the required bug has landed, can you test this out and give me feedback so that we can proceed here?

Comment 5

[:aceman]

Axel, can you please look at this now?

Comment 6

[Axel Grude [:realRaven]]

(In reply to :aceman from comment #5)
> Axel, can you please look at this now?

I currently cannot compile, I promise I do it on Friday evening! I had already glanced over the code which looked fine to me but I need to compile to give you a full assessment.

Comment 7

[:aceman]

Axel?

Comment 8

[:aceman]

Axel, can you get to this soon?

Comment 9

[Axel Grude [:realRaven]]

(In reply to :aceman from comment #8)
> Axel, can you get to this soon?

aceman, how can I test this without compiling a version of Thunderbird? Compiling on my system is broken so I don't know what else I should do? Just read the patch?

Comment 10

[:aceman]

I've started a try build at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=13e569d08ec3 . You can then retrieve the binaries from it.

Comment 11

[Axel Grude [:realRaven]]

(In reply to :aceman from comment #10)
> I've started a try build at
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=13e569d08ec3 . You can then retrieve the binaries from it.

Thanks. Looking at that I need to go into one of the [B] links to get a binary, correct? I am going to wait for a while if the Win7 32bit build completes otherwise going to test the Windows XP one. Will write a patch in quickFilters to try it out.

Comment 12

[Axel Grude [:realRaven]]

Tried it, and would like to give feedback + (f+) 

I can now open the filterlist and preselect a certain filter.

Comment 13

[Axel Grude [:realRaven]]

Comment on attachment 8429472 [details] [diff] [review]
patch v3.1

worked for me!

Comment 14

[:aceman]

Comment on attachment 8429472 [details] [diff] [review]
patch v3.1

Thanks.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 8429472 [details] [diff] [review]
patch v3.1

Review of attachment 8429472 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok to me. r=mkmelin

::: mail/base/content/FilterListDialog.js
@@ +277,5 @@
> + * The listbox view is scrolled to the corresponding item.
> + *
> + * @param aFilter  The nsIMsgFilter to select.
> + *
> + * @return  True/false indicating whether the filter was found and selected.

nit: true

Comment 16

[:aceman]

Thanks.

https://hg.mozilla.org/comm-central/rev/b4a16725a520

Comment 17

[Kent James (:rkent)]

Comment on attachment 8429472 [details] [diff] [review]
patch v3.1

While this was not requested for TB45, its landing caused a later patch (that was requested) to not apply cleanly, so it was easier to accept this than fix the other patch.

http://hg.mozilla.org/releases/comm-beta/rev/a7b82ccc9591
http://hg.mozilla.org/releases/comm-esr45/rev/82112f368aba