179568 - If a message is determined to be "Junk" (spam), just show simple html or plain text - don't load remote images

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[scottputterman]

This is an enhacement suggestion for the junk mail feature.

If a message is classified as Junk, when the user clicks on it, it shouldn't do
java, js, plugins, remote images, etc.  It should just show it in simple html or
plaintext.  The user can view the full message by changing it to not junk or by
using the Message Body as Original HTML menu item.

Comment 1

[Jean-Francois Ducarroz]

I think the way to implement that is to append an attribute to the message url
when parsing it. That would allow mime to do the right thing

Comment 2

[Peter Lairo]

Some suggested Specs:

1. This should be an option under "Tools | Junk mail controls":

| [x] For messages marked as junk, change view to [ Simple HTML \/]  |
|                                                 | Plain Text    |  |
|                                                 +---------------+  |

1.1 Obviously, the default should be *selected* and *Simple HTML*.

2. If the user selects a mail that is marked as "junk", then the "View | Message
Body As" setting is automatically and immediately switched (to Simple HTML or
Plain Text).

3. If the user wants to see the junk mail in all it HTML uglyness, all he sould
need to do is select "View | Message Body As | Original HTML". However, if he
selects *another* "junk" mail, then the pref kicks in again, and he must
reselect "Original HTML" (security & privacy over convenience here).

PS. This bug would take a *lot* of the risk out of confirming ones e-mail
address  to spammers by "accidentally" selecting a junk e-mail (oops). I
consider this bug highly useful and would like to suggest some prioritization
and keywords.

PPS. The OS is "All".

Comment 3

[Peter Lairo]

bug 184948 a dupe of this bug? (this bug was reported first)

Comment 4

[Quinn Yost (mythdraug)]

*** Bug 184948 has been marked as a duplicate of this bug. ***

Comment 5

[Adam Masri]

modifying summary to make this bug easier to find.

- Adam

Comment 6

[Adam Masri]

*** Bug 187803 has been marked as a duplicate of this bug. ***

Comment 7

[Ben Bucksch (:BenB)]

heh, nice idea :)

As the author of both Simple HTML and As Plaintext, I'd highly suggest to choose
Simple HTML. (I save you the triade, reasons on request.)

A message URL attribute would be great, that would help the normal feature as
well to switch on-the-fly for a single msg only, in case we want to add such UI
later, I just didn't do it because I didn't know how.

I don't think we should have a pref to choose between Simple HTML and As
Plaintext for Junk mail. If the user uses Original HTML for normal messages,
then use Simple HTML. Otherwise do nothing (use Simple HTML or As Plaintext,
whatever the user chose for normal messages).

Comment 8

[Ben Bucksch (:BenB)]

From the dup bug:

<http://www.mozilla.org/mailnews/specs/spam/images/Spam5.gif>

re bug 195088 comment 2: It should be fairly easy to prevent inline images,
libmime is in control which tags/attributes to allow, and it can know (if you
tell it so), if the msg is junk, so just use a different pref for allowed
tag/attributes in that case. To prevent inline images, just don't include
"img(src)".

Comment 9

[Ben Bucksch (:BenB)]

*** Bug 195088 has been marked as a duplicate of this bug. ***

Comment 10

[tor]

A third choice for that menu including original Plain Text, Simple HTML, and the
new third on *NOT AT ALL*.   

There is a lot of junk that doesn't get marked as junk.  If I want to view the
message I can turn the "Marked as junkmail" off of that message to view it or
other people can select Simple HTML or Plain Text.  Otherwise for me and I'm
guessing quite a few others, when I click on it I just wanna delete it without
openning and without having to turn off the view message pane.

Partially quoted from comment #2: 
"1. This should be an option under "Tools | Junk mail controls":

| [x] For messages marked as junk, change view to [ Simple HTML \/]  |
|                                                 | Plain Text    |  |"
|                                                 | *NOT AT ALL*  |  |
|                                                 +---------------+  |

Comment 11

[tor]

Slight mod to my last comment: #10

When in "Not at all" mode where the message isn't viewed upon clicking it you 
should be able to view the message by double clicking it and having it open in 
a new window, but a single click (like selecting it) for the purpose of 
deleting the message shouldn't render anything in the message at all.  

(This feature would be useful for deleting spam but also for deleting messages 
the user knows are virus ridden because of the subject and attachment status 
without having to even worry about looking at it.  The click of the Junk Status 
field, highlight the message, delete key... and your done with it.)

Comment 12

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

note, this is covered by jglick's spec.

Comment 13

[Ian Brandt]

I too would like an option to have the preview pane disabled when selecting
messages marked as junk.  Collapsing the preview pane every time you want to
select a junk message without seeing it is too awkward and tedious.

I also like the idea of having the option of turning off images, etc. for junk
messages when viewed in the preview pane, or in their own message window.  I'm
guessing this would be a bit more work to implement than simply inhibiting
preview for junk messages, so if I had to choose I'd rather have the no preview
done first as it covers most of my cases (I can almost always tell from the
sender and subject of a message that it's junk, only rarely do I have to view
the contents).

Regards, Ian

Comment 14

[David Grant]

To Ian: are you crazy?  Why would you want to disable the preview pane
completely for Junk Mail.  What if the subject line is "Hey, how's it going Ian"
and the From: address is nothing too suspicious...don't you want to check it
quickly to see if it's from a p0rn site or from a friend?  I think showing the
Junk mails as plain text is a good idea though.

Comment 15

[orion x elenzil]

If i had the option of disabling display altogether for junk mail,
i would use it. 99% of the time i can tell by looking at the From: and Subject:
fields whether or not it's spam, and with the 1% i'm not sure of, i'd be able to
hit the "Display" button and view the Body.

Displaying it as text-only is a nice second choice for me.

I think in text-only it should probably Not Display any mime-encoded attachments,
even as text, as they're likely to be large, and will slow down the daily task
of identifying the few spams which the filters miss.

Comment 16

[Ian Brandt]

To David: No, as a matter of fact I believe I am perfectly sane.  As I said the
scenario you describe happens to me on only a very, very small percentage of the
marked as spam mail that I receive, and in those cases I don't mind double
clicking the message to open it in it's own window (or even better, clicking a
display button if one is available).  If a message truly is spam then I no more
want to see the text than I do the images.  When I do choose to view a message
questionably marked as spam I would like to see a text only version, but as this
case is so rare I'd rather see the "Inhibit preview pane for messages marked as
junk" option implemented first.  You at least read that I said "option", no?  If
you think the behavior is crazy than you of course wouldn't have to opt for it. 

Comment 17

[tor]

Ian and David:

Why not have it thus:  Messages flagged as Junk do not get viewed when you
single click them so you may delete them without having to render ANY part of
the message.

(This will help guard against viewing spam, porn, and virii infected email.)

And *IF* you do want to view the message you can either take the junkmail flag
off with a single click and wha-la your message shows up or just double click
the message and view it in a new window.

This is an important feature to protect against a variety of bad mail.  There is
junkmail out there that will corrupt your mailbox though the mozilla team works
hard to make their mail program robust there will always be spam out there that
pushes mail programs to the test.  

There are also virii writing folk around that given any little bug in html (even
simple html) they may find can write a virus that will take advantage of your
mail client... It is best if the most bare minimum is processed from the message
when you highlight it to delete it and if it isn't spam by all means just toggle
the junkmail flag on it with a quick click.

thanks.

Comment 18

[Jacek Piskozub]

Tor: You can delete any messages without opening them by using the context menu
(right-clicking on them). You do not need a new and completely un-intitive
mechanism for that.

Comment 19

[tor]

okay...that works for 1 message.  What works for 113 junk out of 150 total
messages ?   Turning off the preview pane manually...  selecting just the junk,
deleting, turning pane back on.   Tedious period.  This is especially tedious
considering that the junkmail filters work pretty well and already flag all
those messages as junkmail automatically... now if there was an option that I
could turn on that would mean that junkmail wasn't rendered I could select
messages all at once and delete just the spam and leave all the regular mail in
place without rendering a single message.   Right click only seems to work on
one message at a time.  Do that 113 times...

Comment 20

[David :Bienvenu]

to mass delete lots of junk mails, you can do this:

sort by the junk mail column by clicking on the column header
extend select all the junk mail
press delete

When I don't want to select any junk mail messages, I start the selection off
with the last non-junk mail, extend select to the end of the junk mail, and then
deselect the non-junk mail. 

Comment 21

[Peter Lairo]

OK, all points have been abundanly made. Comment #10 is pretty clear. Most posts
after that were mostly spam. Please stop spamming this bug. The owner will
decide what is best.

Comment 22

[Jacek Piskozub]

Tor: If you really need to delete 113 messages (instead of waiting for their
natural death after the number of days you set in Tools -> Junk Mail Controls)
you can mark them using Ctrl-LeftClick and then press Delete. As simple as that.

Sorry for the spam.

Comment 23

[Andre-John Mas]

Maybe another approach would be to have an option that would allow images not to
be shown in any messages and then show them when you click on a 'load images'
button (on a per message basis). This feature was available as a global option
in older versions of the navigator. It would be nice if it could be reintroduced
for the mail client.

Comment 24

[Ben Bucksch (:BenB)]

FYI, neither as plaintext nor Simple HTML show any images apart from those
embedded in the mail, which is hardly ever true for spam (I haven't seen that yet).

Comment 25

[Andre-John Mas]

I am reading a piece of spam, which has an image in it, I and when I see the
source I see that it has an external image reference:

<img border="0" src="http://204.188.76.85/LIVE_FROM_THE_WIRE.gif" width="427"
height="55">

This image is getting displayed. If you want I can send the source of the spam
as an attachment to this bug report, or at least a working mock-up.

BTW I am using Mozilla 1.3 when I make this observation

Comment 26

[Andre-John Mas]

Sorry, I hadn't realised my mail client was in 'Orginal HTML' mode. As  Ben
Bucksch, and others, indicated in the other modes the images don't appear. I
just hadn't realised that there was a specific option in the menu for that. I
had been looking in the preference dialogues. Am I alone in being caught looking
in the wrong place?

Comment 27

[phil]

in response to comment 23 I've filled bug 203176.

Comment 28

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

taking.

looks like benb has some good ideas.

this would be good for 1.5 / thunderbird.

Comment 29

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

too bad this won't make 1.4 final.

Comment 30

[Jason M. Abels]

I have a further suggestion on how this could work.

It's been suggested that the message not be shown *at all* if it's determined to
be junk. This would keep some of the more... shocking messages off the screen
entirely.

Instead, the user could be presented with following buttons/choices in the
message pane.

1. Delete this junk message (send to junk mail folder, depending on other settings).
2. View this junk message (in either PlainText or SimpleHTML)
3. This message is not junk.

If the user chooses #2, they would see the message as exists now, with a button
at the top to mark it as not junk. If they choose #3, then they would see the
message as it should appear.

Comment 31

[Tom Sommer]

Re: Comment #30 
This is actually a good idea. Most people can tell if a mail is indeed junk just
by looking at the subject.

But the mail should still be moved to any junk folder (or deleted) based on the
settings in Junk mail controls. It is only after the basic filters have been run
that these extra action buttons should be displayed. Just like the current "Not
junk" button.

The general idea is obviously that junk messages should not load remote images,
since many spammers use that to confirm email's validity. I'd much rather see
the orginal report resolved and then improve the feature afterwards (like you
suggest). This basic feature is essential and needs to be implemented ASAP.

Comment 32

[tor]

A fair amount of mail resides on other folders that is indeed spam the filter
didn't catch.

Marking it as junk should engage a mode like mentioned in comment #30

This would apply to all "junk" even stuff that doesn't get filtered properly and
virus laden email which get marked as junk manually.  It would be really nice to
not render the message at all and protect your computer system by not even
loading the message or any scripts/embedded contect it may have.

Comment 33

[Ben Bucksch (:BenB)]

> virus laden email [...] It would be really nice to
> not render the message at all and protect your computer system by not
> even loading the message or any scripts/embedded contect it may have

This is exactly the point of Simple HTML, to protect your computer. There is no
significant risk involved in using it with nasty email, practically *all* virii,
worms, scripts, remote content, etc. are deactivated. There is no need to
display nothing, you can pretty safely use the Simple HTML mode, that's what
it's made for.

Comment 34

[tor]

Does it hurt anything to have it as a choice to "not display at all" along with
SimpleHTML?    I've seen virus infected image files where the extension of the
file was still .gif and it did have a virus in it.  Virii writers are getting
sneakier and sneakier and using multithreaded attacks that look for multiple
vulnerabilities.  Mozilla is safe... for now.  What about a year from now?

I've also seen junkmail totally lock a browser when it tried to render.  Even
with simple HTML it will try to render some of the mail message.  Does it hurt
anyone to have a choice?   I and most of the other staff here in tech support
would easily choose to not render a message at all if we know it is spam or has
a virus in it based on subject and the size of the mail message.

If it is a choice then folks who do want the message partially rendered can
choose it.  If folks don't want the message rendered at all they can choose that
option all without having to turn off their whole preview pane to delete one (or
133) junk mail.

I don't understand the resistance to having this as a choice.

Comment 35

[Ben Bucksch (:BenB)]

> I've seen virus infected image files where the extension of the
> file was still .gif and it did have a virus in it.

An EXE masking as gif? That would be harmless in this case. Or a buffer overflow
exploit? Working with Mozilla, in the wild? I've never heard about one, and I am
in the Mozilla security team.

If you are seriously worried about such unlikely cases, *this* is the wrong way.
Junk mail detection doesn't catch all mails, it's far more likely that one slips
through these filters. If you are so super-paranoid to worry about such
hypothetical cases, you should enable some of the paranoid settings for all
mails, as described in <http://www.bucksch.org/1/projects/mozilla/108153>.
Otherwise, Simple HTML is for you.

> Does it hurt anything to have it as a choice to "not display at all" along
> with SimpleHTML?

We'd need a pref, while we otherwise need no pref at all. Every UI pref comes at
a cost.

That said, it's futile to discuss about the bug without having anyone to
implement it.

Comment 36

[Steve Wardell]

Let me try to hit the two areas of this. If all options were enabled to view the
message, allow it to execute javascript, and view images I believe we can fairly
safely say no damage can be done to the local system. The security issues arrose
in Outlook because of the system interaction abilities that were available to
email messages without a user even open an attachment.

The intent of this bug has nothing to do with security (except where a user who
sent you email knowing your IP can be considered a security risk, but rather
just a privacy one). This bug attempts to fix the privacy issues in messages
determined to be junk.

As for adding configuration items, I would tend to say that it is outside of the
scope of this bug. 

Personally, I would prefer against such an option of not rendering the message
due to development time as well as additonal configuration and code complexity
with such development. However, if you consider it important please file a bug
for such an option if there is not such an existing bug.

Comment 37

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

might be coming to a tbird near you.

Comment 38

[Scott MacGregor]

Attachment: the test patch thunderbird will be using

Just to elaborate on Seth's comment. 

In the next round of Thunderbird builds, Thunderbird will be sanitizing HTML
for junk messages by default. Here is the match to mimei.cpp which implements
this feature for thunderbird. The ifdef may get turned off in the future,
turning it on for seamonkey mail as well if the feature gets good feedback.

What isn't shown in this patch is the change to the thunderbird junk mail
controls dialog to add a check box (defaulted to on by default) for:

"When displaying HTML messages marked as junk, sanitize the HTML"

This checkbox toggles the pref:
pref("mailnews.display.sanitizeJunkMail", true);

Comment 39

[Ben Bucksch (:BenB)]

Thanks, mscott, for implementing that.

I was looking forward to you adding a parameter to the msg url as mentioned in
comment 1 and comment 7, because that would have allowed some other nice
features as well (I don't remember what I was thinking of concretely, apart from
temporary manual mode switching). I see you didn't do that, but implemented that
in mimei.cpp - it's much easier to implement, so the right thing from your
perspective, I still appreciate the patch.

> add a check box

Why do we need a UI pref for this at all? Sanitize HTML displays enough of the
body to allow the user to determine, if it's spam or not, right? If he then
decides that it's spam, he deletes it via the junk mail button. If he decides
that it's not spam, he can mark it so and it will display the way normal
messages display.

Some comments about the code:
- I don't understand the comment just above html_as = 3;. Is that a leftover
from when you didn't have the pref checking implemented yet?
- You set html_as unconditionally, disregarding the other relevant prefs the
user set. With your patch, if my pref is to View As Plaintext, spam would still
display as Sanitized HTML, and I don't think that's what users expect. If you
want to implement what the last paragraph of comment 7 suggests, you should write
      if (html_as < 3)
        html_as = 3; // 3 == Simple HTML
instead.

I applied the patch (minus pres, plus the additional html_as check) to the
upcoming Beonex Communicator and will test it there.

Thanks.

Comment 40

[Ben Bucksch (:BenB)]

>       if (html_as < 3)
>         html_as = 3; // 3 == Simple HTML

that's nonsense, rather use:

      if (html_as == 0)
        html_as = 3; // 3 == Simple HTML

Comment 41

[Scott MacGregor]

Thanks for the feedback Ben.

I suspect many of the other feaures you wanted the junk paramter for in the url
can be handled in the way I did it here. Use the generic routine I wrote for
pulling out the MsgDBHdr for the url. With the DBHdr you can get/set all sorts
of interesting fields on the message we are trying to render.

>I don't understand the comment just above html_as = 3;. Is that a leftover
>from when you didn't have the pref checking implemented yet?

--> Oops your right, that's a left over comment from before I had actually
hooked up the preference.

>You set html_as unconditionally, disregarding the other relevant prefs the
>user set.

--> Good point Ben. I'll make that change.

Comment 42

[Ben Bucksch (:BenB)]

> I suspect many of the other feaures you wanted the junk paramter for in the url

heh, seems like we interpreted what was said in a different way. I was looking
for a way for the frontend to tell libmime that the message should be rendered
as simplehtml, and the feature here then would be implemented in code nearer to
the frontend. That would have allowed the frontend to switch a certain message
display (not the global, persistant pref) to simple html. I am also concerned
about (inline?)forward/quoteing of HTML msgs, where we should probably use
simple html as well for security reasons, but maybe we can solve that with
mimei.cpp.

Comment 43

[Ben Bucksch (:BenB)]

To make the behaviour more transparent, mayb you should issue a reload of the
message, so that it immediately switches to Simple HTML mode when the user marks
a displayed message as junk? It would be slower, but it would remove the
inconsistence that the message keeps displayed as Original HTML (although it was
just marked as junk), but will be displayed as Simple HTML the next time the
user opens it.

Comment 44

[Ben Bucksch (:BenB)]

Alternatively, marking a message as Junk could act like Delete in that it closes
the message and opens the next one. Then you wouldn't have to reload and it's
probably even what users expect / find useful.

Comment 45

[Scott MacGregor]

About: http://bugzilla.mozilla.org/show_bug.cgi?id=179568#c43, that's how
thunderbird behaves today. Toggling the junk status on a message reloads it and
you can see the HTML alter in both directions.

Comment 46

[Ben Bucksch (:BenB)]

Ah - I am using it with Seamonkey's Messenger 1.4.

Filed bug 211495 about comment 44.

Comment 47

[Ben Bucksch (:BenB)]

I forgot to mention: the patch works great for me (apart from what I mentioned
above). r=BenB

Comment 48

[Ben Bucksch (:BenB)]

Ops, I was a bit fast with that r=, I didn't remember the other parts of the
patch. Looking over it, I am wondering about this:

+      (void) msgHdr->GetStringProperty("junkscore", getter_Copies(junkScoreStr));
+      if (atoi(junkScoreStr.get()) > 50)

<http://www.hh.se/stud/d98rolb/ansi/atoi().html> says "No check is made for
integer overflow". Do we have a potential exploit here, if the incoming message
already contains the relevant header? Maybe use some nsString functions (which
are *hopefully* safe, but check it)?

Comment 49

[David :Bienvenu]

Ben, your comment is specious. That property comes from a junk mail plugin, not
from an RFC822 header. So, no.

Comment 50

[Ben Bucksch (:BenB)]

Yes, I am aware of that, but I thought (because that seems to be in the same
arraz as the incoming RFC822 headers) that there might be a way the sender can
set the junk control header (e.g. when junk controls are disabled). If there is
no way that header can be set from the outside, and there never will be, then my
comment is void, yes. But I rather look silly then to leave a security hole in
there :).

Comment 51

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

mscott, has this made it's way onto a tbird nightly?

it would be nice for 1.5, but not necessary for 1.5 alpha.

should I re-assign to you?

Comment 52

[Ian Brandt]

Will this bug be closed once Scott's patch is in?  In other words, should those
of us in the "Not At All" camp (a la Comment #10) open a new RFE?

Regards, Ian

Comment 53

[Jan Vlug]

When will this being checked in for Mozilla?

Comment 54

[Alfonso Martinez]

*** Bug 223021 has been marked as a duplicate of this bug. ***

Comment 55

[Søren Friis-Nielsen]

The image at http://www.mozilla.org/mailnews/specs/spam/images/Spam5.gif shows
the wanted feature in Netscape 7 but I'm using mozilla 1.5 and it's not there.
Is any work done on this? I am I missing something?

Comment 56

[alge]

Wouldn't "porting" the "[ ] When displaying HTML messages marked as junk,
sanitize the HTML"-uipref from tb fix this for the appsuite?

Comment 57

[Felix Salfner]

Another solution would be to turn off the message pane for the Junk mail folder.

Comment 58

[Jacek Piskozub]

Felix: No! This way it would be virtually impossible to check whether a message
marked as Junk is a legitimate one. This is crucial while you teach the filter
and important long after that. Last week I was milliseconds from marking as read
(and forgetting) an important message wrongly marked as junk.

Comment 59

[Alfonso Martinez]

*** Bug 237462 has been marked as a duplicate of this bug. ***

Comment 60

[Mike Cowperthwaite]

See also bug 195006.

Comment 61

[Ben Bucksch (:BenB)]

*** Bug 195006 has been marked as a duplicate of this bug. ***

Comment 62

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

I think mozilla thunderbird already has this feature.

Comment 63

[Steve Wardell]

In firefox it seems to work, but in today's Suite 1.8a nightly that does not
seem to be the case. On messages marked as Junk when remote image display is not
turned off, the remote images are loading. On Firefox, the remote images do not
load on junk messages (even if remote image loading is not disabled). Can we get
this behavior applied to the suite? Are there other aspects of this issue
missing from the suite that are in the fox?

Comment 64

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

I think tbird does this, and we'd have to back port the content policy manager
to seamonkey if we wanted this for 1.x

Comment 65

[Ben Bucksch (:BenB)]

Seth, yes, Thunderbird has this implemented, see attached patch, and the same
relatively small patch works for seamonkey, I am using it in Beonex Communicator
since then. I can attach the patch I am using (incl. fixes for my comments), if
you want.

Comment 66

[Frédéric Buclin]

Would be great. No need to block remote images for all mails then.

Comment 67

[David Fraser]

In Thunderbird 1.0, it has the option "When viewing mail marked as junk,
sanitize the HTML", which I have checked. However, this still displays inline
images in junk mail in the preview window, so it seems like this patch has not
been implemented fully or has been partially reverted ... am I wrong?

Comment 68

[David Grant]

re comment #67, worksforme

Comment 69

[Mike Cowperthwaite]

(In reply to comment #67)
> In Thunderbird 1.0, it has the option "When viewing mail marked as junk,
> sanitize the HTML", which I have checked. However, this still displays inline
> images in junk mail in the preview window, so it seems like this patch has not
> been implemented fully or has been partially reverted ... am I wrong?

"Sanitized" (a.k.a. Simple HTML) does not suppress display of images that are 
attached to the message; it prevents external images (and other external 
entities) from being loaded, and it suppresses most of the formatting (colors, 
fancy layouts, etc).

Comment 70

[David Fraser]

Re comment 69:
so if I don't want my Junk mail to display inline images, should I open another
bug? A lot of the earlier discussion of this bug included showing something as
plain text instead of sanitised HTML as an option, I'd like that a lot.

Comment 71

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

> so if I don't want my Junk mail to display inline images, should I open another
> bug?

I've logged #336022 on that issue (for tbird).

Comment 72

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Mostly just removing some ifdef MOZ_THUNDERBIRD...

Comment 73

[[e-mail]]

Comment on attachment 398954 [details] [diff] [review]
proposed fix

Thanks for doing this!

> // Overrides of the seamonkey suite mailnews.js prefs
[s/seamonkey suite/core/ ;-) ]

>+      let isJunk = (junkScore == Components.interfaces.nsIJunkMailPlugin.IS_SPAM_SCORE);
Why not use the current junk bar state? (Or !the old junk bar state)

>+      // If the current row  isn't going to change, reload to show sanitized or
Nit: double space between row and isn't

>+      if ((!isJunk && folder.isSpecialFolder(Components.interfaces.nsMsgFolderFlags.Inbox)) ||
>+          (isJunk && !folder.server.spamSettings.manualMark) ||
>+          (isJunk && folder.isSpecialFolder(Components.interfaces.nsMsgFolderFlags.Junk)))
Nit: could write this as
const nsMsgFolderFlags = Components.interfaces.nsMsgFolderFlags;
if (wasJunk ? folder.isSpecialFolder(nsMsgFolderFlags.Inbox)
            : folder.isSpecialFolder(nsMsgFolderFlags.Junk) ||
              !folder.server.spamSettings.manualMark))
Nit: failed to take IMAP delete model into account.

Comment 74

[Karsten Düsterloh]

Comment on attachment 398954 [details] [diff] [review]
proposed fix

Thanks for patching us up!

Comment 75

[Magnus Melin [:mkmelin]]

(In reply to comment #73)
> >+      let isJunk = (junkScore == Components.interfaces.nsIJunkMailPlugin.IS_SPAM_SCORE);
> Why not use the current junk bar state? (Or !the old junk bar state)

Yeah that works too.

> >+      if ((!isJunk && folder.isSpecialFolder(Components.interfaces.nsMsgFolderFlags.Inbox)) ||
> >+          (isJunk && !folder.server.spamSettings.manualMark) ||
> >+          (isJunk && folder.isSpecialFolder(Components.interfaces.nsMsgFolderFlags.Junk)))
> Nit: could write this as
> const nsMsgFolderFlags = Components.interfaces.nsMsgFolderFlags;
> if (wasJunk ? folder.isSpecialFolder(nsMsgFolderFlags.Inbox)
>             : folder.isSpecialFolder(nsMsgFolderFlags.Junk) ||
>               !folder.server.spamSettings.manualMark))

I think i'll leave this as it was, as i find that easier to read.

> Nit: failed to take IMAP delete model into account.

Yeah, will leave an xxx comment for that...

Comment 76

[Magnus Melin [:mkmelin]]

changeset:   3839:c42990e3e9b9
http://hg.mozilla.org/comm-central/rev/c42990e3e9b9

->FIXED