363982 - Ensure correct Account is selected after Create Filter From Message

Status: RESOLVED FIXED

(Thunderbird :: Filters, defect)

Description

[Mike Cowperthwaite]

Create Filter From Message will automatically open the Message Filters dialog if needed in order to show the new filter.

If the Message Filters dialog is already opened when CFFM is executed, and the dialog's Account dropdown is set to the account containing the new filter, then the list updates so you can see the new filter.

But: If the dropdown is set to some other account, it remains there after the CFFM is complete.  If the user wants to verify the creation or run the filter immediately, she needs to change the account.  See bug 198597 (which I'm setting to depend on this bug).

Comment 1

[:aceman]

Does this still hold? There is e.g. bug 543419 that says the opposite. When Message Filters dialog is already open, it is not updated to show the new filter, regardless of which account is selected there.

Comment 2

[:aceman]

I think I can see the problem.
In  OpenOrFocusWindow(), if the window is already open, we could still somehow pass the 'args' to it (e.g. via refresh(args)) and then act upon them in the window.
I'll play with that once bug 543419 lands.

Comment 3

[:aceman]

Attachment: patch

Allows to send a "folder" argument via refresh() too.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8413361 [details] [diff] [review]
patch

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

::: mail/base/content/FilterListDialog.js
@@ +10,5 @@
>  
>  let gFilterListMsgWindow = null;
>  let gCurrentFilterList;
>  let gCurrentFolder;
> +let gSelectedFolder = null;

this isn't necessary is it?

@@ +166,5 @@
>  
> +  if (!processWindowArguments(aArguments)) {
> +    // If we are still in the same account, just redraw the list
> +    // to show new filters.
> +    rebuildFilterList();

we need to rebuild in both cases. with this it's broken if you actually had the right account selected

Comment 5

[:aceman]

(In reply to Magnus Melin from comment #4)
> >  let gFilterListMsgWindow = null;
> >  let gCurrentFilterList;
> >  let gCurrentFolder;
> > +let gSelectedFolder = null;
> 
> this isn't necessary is it?

If window.arguments[0].folder doesn't exist (throws in original code) we set gSelectedFolder = null. As that line is removed, I move it here to the header. Maybe it doesn't matter whether gSelectedFolder is null or undefined, but I better keep the status quo.

> > +  if (!processWindowArguments(aArguments)) {
> > +    // If we are still in the same account, just redraw the list
> > +    // to show new filters.
> > +    rebuildFilterList();
> 
> we need to rebuild in both cases. with this it's broken if you actually had
> the right account selected

We do rebuild. If account is switched, processWindowArguments rebuilds. If it wasn't we rebuild here (as processWindowArguments will return false). But see also patch in bug 1002006. I can pull the change here to make processWindowArguments rebuild in all cases.

Comment 6

[:aceman]

Attachment: patch v2

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 8417026 [details] [diff] [review]
patch v2

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

The case with initially correct account still doesn't refresh to show the new filter for me

Comment 8

[:aceman]

I think I see what you mean. It does not refresh for me too, but randomly.

Comment 9

[:aceman]

Attachment: patch v3

I think I found the problem.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 8419069 [details] [diff] [review]
patch v3

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

Yes, this works! r=mkmelin

Comment 11

[:aceman]

Thanks.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/307f05454f74