65761 - Prefill filter rule with sender of selected message

Status: VERIFIED FIXED

(MailNews Core :: Filters, enhancement, P3)

Description

[laurel]

This bug report is being logged to track to PRD for next release (currently not
in planning document(s), but KMurray will add) request for Message Filters
feature improvement to have a simple mechanism for prefilling a filter rule
based on message selection.

Idea is to have a very stripped-down version of "filters by example" where user
would select,say, a context menu item which would open a new filter rule dialog
set with criteria line set to "sender contains" and populated with the email
address of the selected message's sender.

Other specifications for this feature improvement TBD (but keep it simple).

Older related seamonkey bug for "filters by example" is bug 11036.

Comment 1

[scottputterman]

marking nsbeta1+ and moving to mozilla0.9

Comment 2

[scottputterman]

adding nsbeta1+

Comment 3

[laurel]

cc jglick since we'll need some design and spec coverage on this one.

Comment 4

[Matthew T (active 1999-2002)]

Why isn't this a dup of bug 11036? Apart from the blue-skying about AI filters, 
the two bugs seem identical.

Comment 5

[laurel]

We're currently talking about an extremely basic implementation, whereas the
original idea for 6.0 involved more extensive UI and other implementations.
This simpler bug is more directly tracked to triage talks we're currently having
for our next release.

Comment 6

[Matthew T (active 1999-2002)]

Sorry, if you want a Netscape-only enhancement, please file it in Netscape's own 
bug database. In Bugzilla, `PRD', `6.0', `triage talks', `we', and `next release' 
are meaningless, and so is `other implementations' when we don't even have one 
implementation yet.

The basic implementation you are talking about here is an exact duplicate of bug 
11036 as originally reported. The waffle about AI algorithms etc in that bug is 
typical of what happens to an RFE when it has been open too long, and would not 
need to be implemented in order for that bug to be marked fixed. Further 
enhancements (such as filtering based on multiple messages) could be filed as 
separate bugs depending on 11036.

Please do not file duplicate bugs just for the convenience of one particular 
distributor, because to do so slows down everyone else in the project by making 
them wade through multiple bugs dealing with exactly the same thing.

CCing Asa for sanity check.


*** This bug has been marked as a duplicate of 11036 ***

Comment 7

[scottputterman]

I would agree with you, mpt, if I thought this was an exact duplicate. But I do 
not think it is. There are many examples of taking a complicated bug (such as 
11036) and breaking it into bugs that take on less work.  This is an example of 
this.  This is a bug to simply track creating a filter based on the sender of 
the message.  There's no talk of multiple choices or spam folders.  There is no 
chance that 11036 will be fixed by anyone I know of anytime soon, whereas this 
first step has a chance of being implemented.  We're just doing the exact 
opposite of what you said above. Instead of breaking out the more complicated 
parts in 11036 into a separate bug we broke out the less complicated parts.  I'm 
not going to get into an argument about this.  If you feel very strongly about 
this, then please do the bug filing necessary to make 11036 represent the part 
of the bug we want to fix and then mark as a dup again.

Comment 8

[Matthew T (active 1999-2002)]

> This is a bug to simply track creating a filter based on the sender of the
> message.

Ok, thanks for the clarification. Resummarizing and marking as a dependant of the 
general `filter by example' bug. I'll shut up now.

Comment 9

[esther]

Matthew, we know what were doing when filing bugs.  In fact, Laurel is one of
the most conscientious QA members who identifies duplicates on a regular basis
and most people around here know that.  In the future, please be considerate
when commenting in bugs. 

Comment 10

[scottputterman]

After all of that conversation, marking nsbeta1- and moving to future milestone :-)

Comment 11

[timeless]

helpwanted

Comment 12

[scottputterman]

moving back into nsbeta1+

Comment 13

[laurel]

Since this back in approved nsbeta1 realm, when implemented please provide info
about any rules associated with this prefill feature.  Some questions which
immediately come to mind are:
1.  Would this be unavailable from selected messages in an account type which is
not filterable?  For instance, Local Folders or News.
2.  Would the feature assume the filter to be placed in the selected account or
would it pause at the main filters dialog to allow user to select the account
first?  
3.  This would be unavailable for multiple message selections?
4.  Would this be accessible from a main menu item as well as context menu?

Comment 14

[scottputterman]

moving to mozilla0.9.1

Comment 15

[laurel]

Draft UI spec has been posted to mozilla:
http://www.mozilla.org/mailnews/specs/filters.FilterPrefill.html

Comment 16

[Asa Dotzler [:asa]]

typo in that URL.

http://www.mozilla.org/mailnews/specs/filters/FilterPrefill.html 

Comment 17

[jglick]

Updated spec at:
http://www.mozilla.org/mailnews/specs/filters/FilterPrefill2.html 

Comment 18

[scottputterman]

Mvoing to future milestone.  There's not enough time to do this in nsbeta1

Comment 19

[scottputterman]

reassigning to naving

Comment 20

[Ben Bucksch (:BenB)]

This is a dup of bug 34342, I think.

Comment 21

[scottputterman]

moving to 0.9.6 and reassigning to varada

Comment 22

[scottputterman]

removing helpwanted

Comment 23

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

here's an issue:

if we use the server to enable or disable the "prefill" command, it will be
disabled for mail that is filtered into or read on "Local Folders".

Comment 24

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

about that issue, I'm ok with that behaviour for this release.

the common case:

users read a mail in their Inbox, since it's already filtered.

the inbox is on a server that allows filters.  (general rule, if it has an
INBOX, it can have filters).  So if they use the "Prefill Filter" feature it
will be enabled.

it won't be enabled on news (no news filters yet) or local folders, or imported
mail.  but imported mail is a server-less set local folders, so no new mail will
be delivered.

Comment 25

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

adding link to spec.

from the Issues portion of the spec:

+1.  Would this be unavailable from selected messages in an account type which
is +not filterable? For instance, Local Folders or News. 
+
+Yes, this feature should be disabled for Local and News accounts/messages.

Comment 26

[varada]

Attachment: Patch V1 - Prefill Fitlers

Comment 27

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

does your fix select "is" as the initial rule, instead of "contains"?

see http://www.mozilla.org/mailnews/specs/filters/images/MailFilt2.gif

I'm not sure your change to mailWidget.xml is the way to go, I'll have to 
investigate.

Comment 28

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

met with varada and we've come up with a different way to preflight the new filter.

instead of tweaking mailWidgets.xml, we're going to create a new nsIMsgFilter
from the filter list, create a new search term from the nsIMsgFilter, initialize
the term's attribute, operator and value, and then pass that filter to
initializeDialog()

Comment 29

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

varada also showed me a scenario that he's got a fix for, involving the behavior
of the filter list dialog when it's already open you prefil a filter (and then
hit cancel, or hit ok.)

varada, can you summarize the issue (and your proposal for what it should do) so
that jglick can review it and and once approved, add it to the spec?

Comment 30

[varada]

Attachment: Patch Ver 2. xml changes out; js changes in.

Comment 31

[Håkan Waara]

Varada, nice patch. A comment:
I noticed that you changed the code that focuses an already-existing
FilterEditor to alert the user. I'm not sure this is a good idea. Could you
please 1) focus the existing filtereditor and 2) reuse that window with the new
values like we've done till now?

Comment 32

[Håkan Waara]

Another nit: in FilterEditor.js you added a stale dump()...

Comment 33

[varada]

The reason we dont want to reuse the window is becuase of data loss and the dump
statement will be removed before the final checkin.

Comment 34

[Håkan Waara]

So instead of displaying an alert, can't you just open a new instance of
FilterEditor?  That would be less clumsy.

Comment 35

[varada]

Attachment: Patch Ver 3.

Comment 36

[varada]

We dont support more than one instance of a new filter or edit filter (Filter
Editor) dialog. If there is an existing one that is open and the user clicks on
create a new filter (using prefill)- we have already ruled out opening a new
dialog because of the aforementioned condition. The other alternative would be
to notify the user that he has to finish or be done with the existing dialog
without getting rid of it (to avoid data loss). This I feel is best accomplished
by using a separate alert dialog. I am not too fond of introducing multiple
dialogs but this is not a high frequency issue and this is the optimum course of
action.

Comment 37

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

Comment on attachment 54777 [details] [diff] [review]
Patch Ver 3.

sr=sspitzer

please check with jglick / robinf for the alert text and alert title before checking it in.

Comment 38

[jglick]

>+cannotHaveTwoFilterRulesDialogs=Filter Rules Dialog
>+cannotHaveTwoFilterRulesText=There is already available another open Filter 
>Rules Dialog. Close or complete that first.

Hopefully its rare that the user gets into the situation where they want to 
create a filter based on a msg and the Filter dialogs are already open. Ideally, 
it would be nice if we could just open an additional set of dialogs (Filter 
Rules/Message Filters) if the dialogs were already open as Håkan mentioned.

If that is too difficult to do and we have to go with not allowing this behavior 
for now (maybe we can fix in a future bug), suggested dialog wording:

Title - Should be the product name
Text - The filter cannot be created because the Filter Rules dialog is already 
in use. Please close the dialog and try again.(OK)

This assumes its not an issue if Only the Message Filters dialog is already 
open. In that case, a Filter Rules dialog could open directly on top of the 
existing Message Filters.

Comment 39

[Håkan Waara]

Comment on attachment 54777 [details] [diff] [review]
Patch Ver 3.

The code looks great, but please file a new bug on the multiple FilterEditor dialogs issue before checking this in.

r=hwaara

Comment 40

[Håkan Waara]

One more thing, looks like you added the "Create Filter..." string twice, with
difference accesskeys each time, in your messenger.dtd change. Was that intentional?

Comment 41

[David :Bienvenu]

this patch disables a bunch of commands that shouldn't be disabled (like Edit |
Message as New for local folders).  See bug 106504.

Comment 42

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

a spin off idea, once the regressions are all sorted out:

on drag and drop of a message, if the action is a move and the f key is pressed,
after the drop, pop open the create filter dialog, and pre flight it with the
sender and preflight the action to move to that folder.

Comment 43

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

f + dnd isn't very discoverable. 


but if we remembered the last sender and the last action (move to folder x,
delete to trash, flag, label) we could do "create filter based on last action".

something for a future release.

Comment 44

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

additional bugs: 

#106518 (To: filter creation does the wrong thing)
#106520 (enable problems with "Create Filter..." context menu item in address popup)

Comment 45

[varada]

Attachment: Patch for regressions in prefill filters

Comment 46

[varada]

attachment id 54965 deals with the following.
1- Two separate command ids for the menu and the popup context for creating
filters. This is because the menu uses only the From address whereas the popup
context can use any email address.
2- The Menu and Popup context items are disabled for the local folders as well
as news folders.

Comment 47

[varada]

seth can you take a look at the latest patches. I would like to check these in 
by the end of the day.

Comment 48

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

sorry for the delay

1)

can you explain the difference between "cmd_canCreateFilter" and
"cmd_canHaveFilter"? 

I'd prefer it if the names of those commands clearly indicated what they were for.
(one's for currently displayed message, and one's for the current selected
account, right?)

2)

why do you need to add .emailAddressNode in one case, and not both?

Why isn't it:

if (gCollapsedHeaderViewMode)
  emailAddressNode = document.getElementById("collapsedfromValue").emailAddressNode;
else
  emailAddressNode = document.getElementById("expandedfromValue").emailAddressNode;
  
3)

it's hard to tell from your patches if the changes to the switch statements are
correct.  double check and make sure that you didn't break any commands.

Comment 49

[varada]

Attachment: Final patch with fix for 108606 as well

Comment 50

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

this part of the patch doesn't look right:

-      case "cmd_canHaveFilter":
+      case "cmd_createFilterFromPopup":
+				break;        
+      case "cmd_createFilterFromMenu":
         MsgCreateFilter();
 

Comment 51

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

varada explained privately that no one executes that command.

the oncommand for the xul that observes that command does "CreateFilter
(document.popupNode)" directly.

he's going to add a comment before checking in.

sr=sspitzer

Comment 52

[varada]

Marking Fixed.

Comment 53

[laurel]

OK using nov14 commerical trunk build: mac OS X, win98, and linux rh6.2.
Basic implementation in.
Menu item launches to rules dialog prefilled as appropriate:
  --  with proper filter name based on From email address
  --  criteria selected Sender Is <emailaddress of From>
  --  when action applied and OK'd, filter is added to filter list as enabled
Context popup does all actions as mentioned above, but operates on any selected
header and appropriately fills in filter name and criteria value as the selected
header's email address.
Both menu and context popup operate in both 3-pane and standalone message window.

Any specific problems found with this feature will be logged separately.

Marking verified.