194273 - move to junk folder (or delete) when the user manually marks as junk.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

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

move to junk folder (or delete) when the user manually marks as junk.

See http://www.mozilla.org/mailnews/specs/spam/images/Spam1d.gif

Comment 1

[Jon Granrose]

adding dependency on 11035 so it's easier for people to find this bug.

Comment 2

[Peter Lairo]

There should be a delay of a few seconds in case the user accidentally marks an
important mail a junk. The delay should be about 5 sec.? user settable?

I user selects to "delete" manually marked mail, then the "Mark as Deleted"
pref, if set, should be respected.

Comment 3

[Peter Lairo]

related/equal to bug 180029?

Comment 4

[Jon Granrose]

similar but different.  that bug is so if you choose "run jmc manually" it will
move the mail it marks as junk to the junk folder.  this is so if jmc doesn't
mark a message as junk, then when you mark it as junk yourself, it goes away
automatically.  that one, jmc marks it as junk, this one, you mark it as junk.

Comment 5

[Peter Lairo]

This is a dangerous feature. Perhaps the junk icon could blink 5 times before
the mail disappears into the (potentially very full) junk folder. A pulsating
yellow outline around the small junk icon in the messages pane would be cool.

Comment 6

[Alec Flett]

or make it a pref - it drives me NUTS that I have to hit junk-delete to delete a
message. 

The only thing that makes this really dangerous is that a good message can get
mistakenly marked as junk.. and then since its no longer on-screen, the user
can't easily undo that.. but wait! There is an "Undo" file menu item.. that
solves the problem for at least some of the users.

How about a confirmation dialog "Are you sure you want to mark this message as
Junk" (someone else can write the copy) with one of those "Don't show this to me
again" checkboxes?

Comment 7

[jglick]

Plan is make it a pref, Move, Delete or Off completely.
http://www.mozilla.org/mailnews/specs/spam/#JMC

Comment 8

[Samir Gehani]

Mail triage team: nsbeta1+/adt2

Comment 9

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

accepting, beta (or alpha), but most likely beta.

Comment 10

[parish]

FWIW, my preference (no pun intended) would be for a button (make the Junk
button a drop down?) and/or a menu (including context) item to "Move all junk to
Junk folder", where 'Junk folder' is the one defined in the JMC. My reasons are,
as mentioned in other comments, the danger of accidently marking a good mail as
junk and not noticing because it disappears.

Lets be realistic here; this is only an issue in the early days of training JMC,
once it's up and running (mine is now ~90% accurate) the number of messages
involved is small so a couple of extra mouse clicks isn't a major usability issue.

Just my €0.02 of course.

Comment 11

[Jon Granrose]

To present the flipside: I average about 40 spams a day at work, and about the
same at home and JM catches about 90%.  So I average 8 uncaught spam per day and
they're spread across multiple accounts and/or folders which means I do this
manual move many times per day.

Not a day goes by that I don't hope to see a comment in this bug saying this is
fixed.  IMHO, this is the one feature left to make the JMC complete (and perhaps
the "disable images, js, etc. when showing junk mail" bug) after which I will
convert all my friends to the pleasures of mozilla 1.4b and junk mail filtering...

Have I mentioned Seth's my hero... :-)

Comment 12

[parish]

OK, I take your point about multiple mailboxes; mine all come into one so it's
easier. In that case, I'll go along with Alec's comment 6 suggesting pref
controlled.

Comment 13

[David Grant]

Should the target be changed to 1.4b, since 1.4a is already out?  Thanks.

Comment 14

[Rafael Ebron (:rebron)]

adt: nsbeta1+/adt1 

Note to UI/l10n folks: this fix entails a UI change.

Comment 15

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

aiming for 1.4 final

Comment 16

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

for 1.4, I'll probably just to part of
http://www.mozilla.org/mailnews/specs/spam/images/Spam1d.gif, not the whole change.

Comment 17

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

Attachment: what I plan on doing for 1.4 final (improvements to come later, in 1.5)

Comment 18

[robinf]

For the help docs, I can absorb this change before my deadline.

Comment 19

[bobj]

For the UI change: r=bobj.
Added rcloseirl to the cc.  yxia was already on the cc.

Comment 20

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

Attachment: ui fix, still need to do the actual move / delete

Comment 21

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

Attachment: screen shot

Comment 22

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

Attachment: patch

still a work in progress, but the basics work

some todo items:

1) marking as from search (advanced and QS)? (delete / move from search)
2) test various imap delete models
3) UI (and back end), worry about aol and netscape
4) local folders issue (use default server settings?)
5) worry if delete or move is disabled?
6) handle batch 
7) what if in trash or in junk folder already?
8) selecting the next message
9) test undo
10) fix if toggle using thread pane, no selection

Comment 23

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

Attachment: better patch, still testing / working on this.

Comment 24

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

Attachment: patch, no selects the right message after a move / delete

Comment 25

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

Attachment: patch

1) marking as from search (advanced and QS)? (delete / move from search)

works.

5a) worry if delete or move is disabled

see GetCanDeleteMessages()

6) handle batch mark

works.

7) what if in trash or in junk folder already?

works

if in trash, and we say delete, we don't delete
if in junk, and we say move, we don't move

8) selecting the next message

fixed.

9) test undo

works.

10) fix if toggle using thread pane, no selection

if not toggled thing not selected, select, then run command

todo list for tomorrow:

2) test various imap delete models
3) UI (and back end), worry about aol and netscape
4) local folders issue (use default server settings?)
5b) worry if move is disabled?

Comment 26

[Peter Lairo]

Will comment #2 (x sec delay before delete) and/or comment #5 (blink x times
before delete) be considered or even implemented in this round?

Where "x" should be some meaningful default. No need for a UI setting here.

Comment 27

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

> Will comment #2 (x sec delay before delete) and/or comment #5 (blink x times
> before delete) be considered or even implemented in this round?

From running my patch, I don't think it's needed, since you can undo.

Comment 28

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

an updated todo list:

2) test various imap delete models
3) UI (and back end), worry about aol and netscape
4) local folders issue (use default server settings?)
5b) worry if move is disabled for the src folder
11) test manual move, but move disabled?

Comment 29

[Peter Lairo]

RE: "x-sec delay before delete"

> From running my patch, I don't think it's needed, since you can undo.

I (slightly) disagree:
1. The undo feature is undiscoverable, and is uncommon in this scenario.
2. People might mark-as-junk before having fully read the header - oops.

Comment 30

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

two issues david found during review:

a)

1,2,3,4 selected, and I toggle the icon for row 1
the move / delete will apply (incorrectly) to all rows

b)

if you classified 20 messages, and selected another message before the
classification finished, those 20 wouldn't get moved / deleted

re-working the patch.

Comment 31

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

Attachment: patch, still need to test.

Comment 32

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

Attachment: patch, testing is going well, I had to fix one bogus assertion

Comment 33

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

Attachment: patch, disable manual mark UI if move is disabled (as the code in nsMsgDBView.cpp won't do the move)

Comment 34

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

Attachment: patch, fix problem in JMC dialog and fix some code, per bienvenu's review (over aim)

Comment 35

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

Attachment: patch, fixed JMC issue (for sure this time)

Comment 36

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

Attachment: screenshot

QA:  note how if auto move feature is disabled, the UI for manually move is
disabled.

Comment 37

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

jon wrote:

> IMHO, this is the one feature left to make the JMC complete (and perhaps
> the "disable images, js, etc. when showing junk mail" bug)

that's bug #179568.

> Have I mentioned Seth's my hero... :-)

even if it took me nearly three months to fix this?

Comment 38

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

fixed landed, r/sr=bienvenu.

I'm going to address the local folders issue next, in a spin off bug, by most
likely making it show in the JMC dialog, assuming that can be made to make sense.

Comment 39

[David :Bienvenu]

one thing I see after building with this patch is that the junk mail controls
dialog needs to be bigger (taller).

Comment 40

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

Attachment: fix, for local folders

Comment 41

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

Comment on attachment 123296 [details] [diff] [review]
patch, fixed JMC issue (for sure this time)

r/sr=bienvenu, a=sspitzer

Comment 42

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

Comment on attachment 123361 [details] [diff] [review]
fix, for local folders

r/sr=bienvenu, a=sspitzer

Comment 43

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

there's one issue (so far) that I need to fix.

an assertion, and a painting issue:

###!!! ASSERTION: selected indices for deletion is 0: 'selectionCount', file c:/
trees/trunk/mozilla/mailnews/base/src/nsMsgDBView.cpp, line 5504

you can get that if you use the thread pane junk status icon and the message
isn't selected.

I'll log a new bug.

Comment 44

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

Attachment: patch, not sure if this is the right approach, for junk status cycler issue

Comment 45

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

> one thing I see after building with this patch is that the junk mail controls
> dialog needs to be bigger (taller).

hmm, let me spin this off.  see bug #205876

Comment 46

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

> ###!!! ASSERTION: selected indices for deletion is 0: 'selectionCount', file c:/
> trees/trunk/mozilla/mailnews/base/src/nsMsgDBView.cpp, line 5504

> you can get that if you use the thread pane junk status icon and the message
> isn't selected.

spun off to bug #205877

Comment 47

[parish]

Is this fix functioning as designed?

I have set the prefs to move maually marked messages but, when I do manually
mark one, the Inbox and Junk folder counts update immediately and the status bar
displays Moving 1 of 1 messages to Junk, but the message still appears in the
Inbox header pane. I find I need to switch focus from the Mail window and back
again for it to disappear.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030515

Comment 48

[Stefan Borggraefe]

parish: I think bug 205877 deals with that problem.

Comment 49

[esther]

Using trunk build 20030519 on winxp, macosx and linux
verified, note: the spin off bug 205877 is still open and cam make this appear
as it's not working.