Prefill filter rule with sender of selected message

VERIFIED FIXED in mozilla0.9.6

Status

MailNews Core
Filters
P3
enhancement
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: laurel, Assigned: varada)

Tracking

(Blocks: 1 bug)

Trunk
mozilla0.9.6
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments)

(Reporter)

Description

18 years ago
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.
(Reporter)

Updated

18 years ago
Keywords: nsbeta1
QA Contact: esther → laurel

Comment 1

18 years ago
marking nsbeta1+ and moving to mozilla0.9
Priority: -- → P3
Target Milestone: --- → mozilla0.9

Comment 2

18 years ago
adding nsbeta1+
Whiteboard: [nsbeta1+]
(Reporter)

Comment 3

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

Comment 4

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

Comment 5

18 years ago
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

18 years ago
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 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE

Comment 7

18 years ago
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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 8

18 years ago
> 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.
Blocks: 11036
Summary: Simple prefilled filter rule based on selection ("by example") → Prefill filter rule with sender of selected message

Comment 9

18 years ago
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

17 years ago
After all of that conversation, marking nsbeta1- and moving to future milestone :-)
Keywords: nsbeta1 → nsbeta1-
Whiteboard: [nsbeta1+] → [nsbeta1+ 2/13]
Target Milestone: mozilla0.9 → Future

Comment 11

17 years ago
helpwanted
Status: REOPENED → NEW
Keywords: helpwanted

Comment 12

17 years ago
moving back into nsbeta1+
Keywords: nsbeta1- → nsbeta1
Whiteboard: [nsbeta1+ 2/13] → [nsbeta1+]
Target Milestone: Future → mozilla0.9
(Reporter)

Comment 13

17 years ago
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

17 years ago
moving to mozilla0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
(Reporter)

Comment 15

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

Comment 18

17 years ago
Mvoing to future milestone.  There's not enough time to do this in nsbeta1
Keywords: nsbeta1 → nsbeta1-
Whiteboard: [nsbeta1+] → [nsbeta1+ 4/12]
Target Milestone: mozilla0.9.1 → Future

Comment 19

17 years ago
reassigning to naving
Assignee: gayatrib → naving

Comment 20

17 years ago
This is a dup of bug 34342, I think.

Comment 21

17 years ago
moving to 0.9.6 and reassigning to varada
Assignee: naving → varada
Keywords: nsbeta1-
Whiteboard: [nsbeta1+ 4/12]
Target Milestone: Future → mozilla0.9.6

Updated

17 years ago
Blocks: 102231

Comment 22

17 years ago
removing helpwanted
Keywords: helpwanted
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".
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.
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.
(Assignee)

Comment 26

17 years ago
Created attachment 53791 [details] [diff] [review]
Patch V1 - Prefill Fitlers
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.
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()
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?
(Assignee)

Comment 30

17 years ago
Created attachment 54720 [details] [diff] [review]
Patch Ver 2. xml changes out; js changes in.

Comment 31

17 years ago
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

17 years ago
Another nit: in FilterEditor.js you added a stale dump()...
(Assignee)

Comment 33

17 years ago
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

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

Comment 35

17 years ago
Created attachment 54777 [details] [diff] [review]
Patch Ver 3.
(Assignee)

Comment 36

17 years ago
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.
Status: NEW → ASSIGNED
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.
Attachment #54777 - Flags: superreview+

Comment 38

17 years ago
>+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

17 years ago
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
Attachment #54777 - Flags: review+

Comment 40

17 years ago
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

17 years ago
this patch disables a bunch of commands that shouldn't be disabled (like Edit |
Message as New for local folders).  See bug 106504.
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.

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.
additional bugs: 

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

Comment 45

17 years ago
Created attachment 54965 [details] [diff] [review]
Patch for regressions in prefill filters
(Assignee)

Comment 46

17 years ago
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.
(Assignee)

Comment 47

17 years ago
seth can you take a look at the latest patches. I would like to check these in 
by the end of the day.
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.

(Assignee)

Comment 49

17 years ago
Created attachment 56771 [details] [diff] [review]
Final patch with fix for 108606 as well
this part of the patch doesn't look right:

-      case "cmd_canHaveFilter":
+      case "cmd_createFilterFromPopup":
+				break;        
+      case "cmd_createFilterFromMenu":
         MsgCreateFilter();
 
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
(Assignee)

Comment 52

17 years ago
Marking Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 53

17 years ago
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.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.