create filter from doesn't create filter while in Unified Inbox, and no error message

RESOLVED FIXED in Thunderbird 41.0

Status

Thunderbird
Filters
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Tyler, Assigned: aceman)

Tracking

31 Branch
Thunderbird 41.0

Thunderbird Tracking Flags

(thunderbird39 wontfix, thunderbird40 fixed, thunderbird41 fixed, thunderbird_esr3839+ fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

1.22 KB, patch
Magnus Melin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; rv:11.0) like Gecko

Steps to reproduce:

While Inbox is selected in Unified view (accounts underneath not expanded), I have a message in view (it is in an IMAP account, I haven't tested one of my POP accounts in Unified view as I don't use them as much) (Vertical View with Folder Pane and Message Pane both Enabled), I click on an/the email address in the From field in the Message Pane (I think.. the window on the right side that you can see the email contents inside of), and choose Create Filter From, which brings up the "Filter Rules" window, with the following Fields:

Filter name: AAA Example Filter
Apply Filter When:
X Manually Run
X Getting New Mail: Filter before Junk Classification
X Match all of the Following
Rule1: From IS bugzilla-daemon@mozilla.org (for example)

Perform These actions:
Action1: Move Message to: Some Local Folder
Action2: Stop Filter Execution

I choose OK


Actual results:

Takes me to the Message Filters window for the first account in my list of accounts (not the account that the message arrived into), listing all the filters for that account, it doesn't show my new filter, and I cannot find my new filter anywhere under any of the accounts or the Local Folders filters.


Expected results:

It should take me to Message Filters window for the account that the message arrived into, listing all the filters for that account.  And it should show the new filter.
(Reporter)

Comment 1

3 years ago
Some possibly related bugs/issues in the past that I read through..

https://bugzilla.mozilla.org/show_bug.cgi?id=537012
https://bugzilla.mozilla.org/show_bug.cgi?id=242665
http://forums.mozillazine.org/viewtopic.php?f=39&t=2796221

Other things of note:
- IMAP may be the issue?
- Unified View may be the issue?
- Unidentified bug of process that should be working?

I can test a pop account's inbox as well if it helps to see if there's any difference and maybe narrow it down to a Unified view issue?
(Assignee)

Comment 2

3 years ago
I can not reproduce the problem. For me the Filters window opens on with the proper account and the new filter is in the list.
So yes, please try to play with it and find what is the necessary setup to see the bug. What type of folder do you try it on? Is it the Inbox of the IMAP account or some other folder? Subfolder of Inbox or not?

Comment 3

2 years ago
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Thunderbird/40.0a1

I have the same problem as the reporter. The account that the message arrived into was a POP account.

UNIFIED VIEW IS SELECTED. 

Once the filter is created the behavior is exactly as reporter describes i.e.
1. Takes me to Message Filters window for the first account in my list of accounts
2. The filter is NOWHERE to be found (in any account)

Question: Is there a way to see ALL filters created together?
(Assignee)

Comment 4

2 years ago
OK, I can see it now. I must be on the top level Inbox, that summarizes messages from all accounts.
The filter is created without any error, but is nowhere to be found.

So what is the proposal? Should it be created in the account where the source message is located?
Assignee: nobody → acelists
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All

Comment 5

2 years ago
(In reply to :aceman from comment #4)
> OK, I can see it now. I must be on the top level Inbox, that summarizes
> messages from all accounts.
> The filter is created without any error, but is nowhere to be found.
> 
> So what is the proposal? Should it be created in the account where the
> source message is located?

In my mind it would be ideal if ONE filter was created for ALL accounts (since the same rule would apply more or less to all and any accounts e.g. IF message is received FROM XXXX then MOVE in YYYY folder).

If that is too hard to accomplish the second best would be as you suggest: The filter should be created in the account where the source message is located. (That was my immediate impulse anyway: Look in the account where the message was received).
(Assignee)

Comment 6

2 years ago
Created attachment 8600269 [details] [diff] [review]
patch

So let us send also the folder of the message. Otherwise we were sending the currently selected folder, which could be the "smart mailboxes" account for unified folders. That one can't be seen in the filters list UI and probably filters in it do not run ever.
Attachment #8600269 - Flags: review?(rkent)
Attachment #8600269 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 7

2 years ago
(In reply to CAK from comment #5)
> In my mind it would be ideal if ONE filter was created for ALL accounts
> (since the same rule would apply more or less to all and any accounts e.g.
> IF message is received FROM XXXX then MOVE in YYYY folder).
We have other bugs filed for having global filters. It is currently not possible.

> If that is too hard to accomplish the second best would be as you suggest:
> The filter should be created in the account where the source message is
> located. (That was my immediate impulse anyway: Look in the account where
> the message was received).
Yes, I implement this version in this quick patch.
Status: NEW → ASSIGNED

Comment 8

2 years ago
Comment on attachment 8600269 [details] [diff] [review]
patch

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

::: mail/base/content/msgHdrViewOverlay.js
@@ +1656,5 @@
>   */
>  function GetNewsgroupServer()
>  {
>    if (gFolderDisplay.selectedMessageIsNews) {
> +    let server = getFolderOfCurrentMessage().server;

folder can be null
Attachment #8600269 - Flags: review?(mkmelin+mozilla) → review-
(Assignee)

Comment 9

2 years ago
(In reply to Magnus Melin from comment #8)
> folder can be null

I guessed it couldn't because of the if (gFolderDisplay.selectedMessageIsNews), and my patch doesn't make it any worse, but I can surely add the guard.
(Assignee)

Comment 10

2 years ago
Created attachment 8603867 [details] [diff] [review]
patch v1.1
Attachment #8600269 - Attachment is obsolete: true
Attachment #8600269 - Flags: review?(rkent)
Attachment #8603867 - Flags: review?(rkent)
Attachment #8603867 - Flags: review?(mkmelin+mozilla)

Comment 11

2 years ago
(In reply to :aceman from comment #9)
> (In reply to Magnus Melin from comment #8)
> > folder can be null
> 
> I guessed it couldn't because of the if
> (gFolderDisplay.selectedMessageIsNews), and my patch doesn't make it any
> worse, but I can surely add the guard.

Ah, I misread that context. Let's not add more guards than needed.

Comment 12

2 years ago
Comment on attachment 8603867 [details] [diff] [review]
patch v1.1

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

::: mail/base/content/msgHdrViewOverlay.js
@@ +1608,5 @@
>  /**
> + * If there is a currently selected message, return its folder.
> + * Otherwise null.
> + */
> +function getFolderOfCurrentMessage() {

... but in short I don't see the use for this function. It's short enough to write gFolderDisplay.selectedMessage.folder + other guards that you usually have anyway

And also, MsgFilters tries to do the auto selection in a very similar way if you pass in null (like we currently do). So, I guess that is the place that should be fixed instead?
Attachment #8603867 - Flags: review?(rkent)
Attachment #8603867 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 13

2 years ago
Created attachment 8608324 [details] [diff] [review]
patch v2

Yes, it seems MsgFilters does the same analysis, we just didn't len it do it and passed the wrong folder.
Attachment #8603867 - Attachment is obsolete: true
Attachment #8608324 - Flags: review?(mkmelin+mozilla)

Comment 14

2 years ago
Comment on attachment 8608324 [details] [diff] [review]
patch v2

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

Great! r=mkmelin
Attachment #8608324 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 15

2 years ago
Thanks.
Keywords: checkin-needed

Updated

2 years ago
tracking-thunderbird_esr38: --- → 39+

Comment 16

2 years ago
https://hg.mozilla.org/comm-central/rev/e7bf02ea3e61 -> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0

Updated

2 years ago
Keywords: checkin-needed

Comment 17

2 years ago
Comment on attachment 8608324 [details] [diff] [review]
patch v2

Seems low risk.
Attachment #8608324 - Flags: approval-comm-esr38+
Attachment #8608324 - Flags: approval-comm-beta+

Updated

2 years ago
status-thunderbird39: --- → wontfix
status-thunderbird40: --- → affected
status-thunderbird41: --- → affected
status-thunderbird_esr38: --- → affected

Comment 18

2 years ago
http://hg.mozilla.org/releases/comm-beta/rev/5c3c2307548b
http://hg.mozilla.org/releases/comm-esr38/rev/6a83ba854b14
status-thunderbird40: affected → fixed
status-thunderbird41: affected → fixed
status-thunderbird_esr38: affected → fixed
You need to log in before you can comment on or make changes to this bug.