If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Integrate Junk Mail settings into "Options" and "Account Settings"

RESOLVED FIXED in Thunderbird2.0

Status

Thunderbird
Account Manager
--
enhancement
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: Michael Favia, Assigned: Scott MacGregor)

Tracking

(Depends on: 1 bug, {fixed1.8.1})

unspecified
Thunderbird2.0
fixed1.8.1
Dependency tree / graph
Bug Flags:
blocking-thunderbird2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 4 obsolete attachments)

29.34 KB, image/png
Details
34.24 KB, image/png
Details
136.72 KB, patch
Scott MacGregor
: review+
Scott MacGregor
: superreview+
Scott MacGregor
: approval-branch-1.8.1+
Details | Diff | Splinter Review
4.46 KB, patch
Details | Diff | Splinter Review
3.72 KB, patch
Bienvenu
: superreview+
Details | Diff | Splinter Review
35.99 KB, image/gif
Details
2.90 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040612 Firefox/0.8
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040612 Firefox/0.8

 === Bug Research Summary ===

After wandering through all of the bugs that have been filed against Junk Mail,
Message Filters, and account settings i have come up with a short summary and an
evaluation of the general attitude for a solution. I hope that other bugs might
be able to be closed (as dupes or somesuch) and added to this discussion instead
as they all seem to be leaning this direction anyway just bits and pieces all over.

   1. Junk mail filtering is per account.
   2. Message Filtering is per account.
   3. Account Setting is per account.
   4. All three of these features are managed and configured in seperate dialogs
currrently.
   5. Most people would like to see them somehow elegantly unified. 

== Bugs evaluated and consolidated ==
   1. Bug 205883
   2. Bug 219138
   3. Bug 180087
   4. Bug 205883


== Proposed Solution ==

I propose solving this UI mess in two steps:

=== Consolidate Junk Mail/Message filters ===

A filter takes an action if a condition is met (E.g. Sender is
Michael@insitesinc.com then move to specified folder). Based on this analysis
junk mail filters are no different (Calculate the probability and if the
probability is above XX then move to specified folder) and should be handled as
such. By making this a normal filter we can even grant the end user some control
over the sensitivity of the Junk filter (if we desire and is sensible).


=== Relocate Unified Message Filter to "Account Settings" ===

Filters (Normal and Junk are now one in same) already are applied per account
but we force the user to manage them seperately from all the other account
settings. By placing the filters under account settings we further simplify the
UI and consolidate functionality intuitively.

=============

If i have guaged public opinion incorrectly please feel free to close this
enhancment bug. I just noticed a lot of individuals with good ideas about a
needed improvement not in the same discussions. Id also like to add a comment to
 each of the bugs referenced in an attempt to unify the discussion about
unifying the configuration but dont want to over produce bugspam. Any suggestion
on ettiquete/appropriateness(sp?)?

PS If there is interest in this type of a solution i will happily produce some
mockups so we can proceed from there.

Reproducible: Always
Steps to Reproduce:

Updated

13 years ago
Component: Preferences → Account Manager

Comment 1

13 years ago
*** Bug 281462 has been marked as a duplicate of this bug. ***

Comment 2

13 years ago
For the junk controls, yes: bug 180087 requests that for Seamonkey.  See also 
bug 205883.  Some of the JMC settings are (or should be) global, and should go 
into the Options dialog instead.

Message filters perhaps should *not* be "per-account" -- there is a longstanding 
request for global filters (bug 34973, bug 129883), and another (bug 267197) 
asking that the account be a criterion of the filter (rather than the filter an 
attribute of the account).

The URL given in the field above is now 404'ing.  Is that document still around?

Comment 3

13 years ago
First cut at partitioning these between global and account-specific settings. 
I'm sure there will be some people who want one or more of the items shown here 
in Options to be set per-account, but this is at least a starting point.  It 
does not address any of the other RFEs for junk controls.

ACCOUNT SETTINGS
 [] Enable adaptive junk mail detection
   [] Move detected junk to 
      () "Junk" on  [account]
      () other....  [folder]
      [] Automatically delete messages older than [nnn] days from Junk folder

OPTIONS (apply to all accounts):
 [] Do not mark messages as junk if the sender is in:
    [address book]
 [] When I manually mark messages as Junk:
      () Move them to the account's Junk folder
      () Delete them

 [] When displaying HTML mail marked as junk, sanitize the HTML

 [] Enable junk filter log     [Show log]

 [Reset Training Data]
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Unification of Junk and message filter under "Account Settings" → Integrate Junk Mail settings into "Options" and "Account Settings"

Comment 4

13 years ago
The new option added for bug 290237:
  [] Trust junk headers for [ISP-junk-detector]
obviously needs to be per-account.
(Assignee)

Comment 5

12 years ago
Mike, do you think it's ok to make folks go to two separate places to adjust
their junk control settings?

We could add a new tab to the privacy panel in the options dialog for the non
account specific junk settings proposed by Mike. That's the same panel where you
can configure handling remote images, anti-virus, passwords, javascript, etc. 

Comment 6

12 years ago
(In reply to comment #5)
> Mike, do you think it's ok to make folks go to two separate places to adjust
> their junk control settings?

Right now, folks have to go to three different places to adjust settings -- one 
for junk, one for account stuff, one for global options.  I agree there is a 
potential for confusion; you can see that now where people go to Options looking 
for the switch to turn off HTML compose.  In both cases, I think there should be 
text on the Options page stating that additional, related settings can be found 
in the Account Settings window.

It's obvious that some of the junk stuff is account-specific.  You could leave 
all of the settings account-specific, I suppose, if you really wanted to 
localize junk controls in a single place -- and I imagine there are some people 
out there who do want account-specific behavior for one or more of the settings 
I proposed as global.

One junk setting is already global:
  mailnews.display.sanitizeJunkMail
but the current dialog's design implies that it is just as account-specific as 
any of the other settings.


> We could add a new tab to the privacy panel in the options dialog for the
> non account specific junk settings

That's a fine idea.
(Assignee)

Comment 7

12 years ago
Well today you only go to the one placed for all things junk control. This
proposoal has you go to two places. I'm not saying I don't like it, just
pointing that out.

I took a quick look at implementing this design change as I thought the UI part
would be pretty easy to do. The complication I ran into was that all of our junk
settings are currently account specific. Each account has one of these spam
setting objects associated with it:

http://lxr.mozilla.org/mozilla/source/mailnews/base/public/nsISpamSettings.idl

So we'd have to change the underlying spam settings implementation and not just
the UI. Then the question becomes what do we do about your current account
setting values when we move you to treating some of them as global settings. 

Comment 8

12 years ago
(In reply to comment #7)
> The complication I ran into was that all of our junk
> settings are currently account specific. [...]
> 
> So we'd have to change the underlying spam settings implementation and not
> just the UI. Then the question becomes what do we do about your current
> account setting values when we move you to treating some of them as global
> settings. 

If it's too much work, don't do it; as I said in comment 6, some people probably 
*want* those settings per-account, and we can always open a (never-to-be-fixed?) 
RFE about actually globalizing the other proposed-global settings.  

The majority of the junk UI can still be integrated into Account Settings; an 
appropriate global Options page is only needed for "Show Junk Sanitized" and 
"Reset Training Data."
(Assignee)

Comment 9

12 years ago
Created attachment 218466 [details] [diff] [review]
work in progress

Saving off some of my work. This patch gets rid of the junk mail dialog, adds a junk mail to the account manager, and adds a junk panel to the privacy pane in the options dialog. I also made several features global settings as well.

screen shots coming up.
(Assignee)

Comment 10

12 years ago
Created attachment 218467 [details]
screen shot of new account manager
(Assignee)

Comment 11

12 years ago
Created attachment 218468 [details]
screen shot of the new options panel for junk settings
(Assignee)

Updated

12 years ago
Blocks: 180087
(Assignee)

Comment 12

12 years ago
Created attachment 218487 [details] [diff] [review]
updated patch ready for review

This patch ties up a few lose ends I hadn't addressed yet. I'll request a review once I type up a summary of all the changes. This patch also includes some seamonkey work (adding some locale files, removing the obsolete junkMail dialog from the UI and the build).
Attachment #218466 - Attachment is obsolete: true
(Assignee)

Comment 13

12 years ago
Comment on attachment 218487 [details] [diff] [review]
updated patch ready for review

Here's what I did:

1) nsMsgAccountManagerDS.cpp.h
Add code to display a junk mail panel for all accounts but news who have identities associated with them.

2) incomingServer
I didn't like how the server was responsible for initializing each spam setting attribute so I pushed all of the initialization code into the spam settings object itself. The spam settings object is now readonly, we don't need a setter because the account manager is now setting the prefs for us.

3) spamSettings
A lot of interface cleanup here, getting rid of unneeded setter properties (we could still clean up more) now that the account manager actually sets our prefs.  I made the following settings global: logging, manual mark, and manual mark mode. Converted the logging code to use nsIFile and removed some logging APIs which we don't need. The log now appears at the top level directory in the profile. Add an initialization method which takes a server and updates the server specific spam settings. I also added code to unset/set the junk folder flag when the junk folder changes.

4) am-junk.xul, am-junk.js
Account specific junk mail settings. 

5) privacy.xul, privacy.js
Global junk mail settings

6) Misc
For the prefs I made global (manual mark mode and logging, I didn't try to migrate account settings data. I don't think it's a big deal for a user to reset these values. I would think differently for some of the other settings (like which junk folder to use). I also removed the UI for sanitizing HTML in junk mail, there's no good reason to expose this via the UI.
Attachment #218487 - Flags: superreview?(bienvenu)
(Assignee)

Comment 14

12 years ago
Comment on attachment 218487 [details] [diff] [review]
updated patch ready for review

Neil, are you interested in reviewing the core changes for: am-junk.*, nsMsgIncomingServer, and nsMsgSpamSettings?

see comment 12 for a description of the changes.
Attachment #218487 - Flags: review?(neil)

Comment 15

12 years ago
Comment on attachment 218487 [details] [diff] [review]
updated patch ready for review

my one concern is about migration from the per-server prefs to the global prefs. What happens in that scenario?
Attachment #218487 - Flags: superreview?(bienvenu) → superreview+

Comment 16

12 years ago
Comment on attachment 218468 [details]
screen shot of the new options panel for junk settings

There seems to be a reasonable amount of space in this panel; would it be possible to display the log inline? If not, I think it would be useful to disable the show log button if the log file does not exist.

Comment 17

12 years ago
Will this include issues noted (and by me) in bug 268101
(Assignee)

Comment 18

12 years ago
(In reply to comment #16)
> (From update of attachment 218468 [details] [edit])
> There seems to be a reasonable amount of space in this panel; would it be
> possible to display the log inline? 

I don't think I like the idea of showing the log inline here. I like having it in a separate dialog, and I expect we'll have some more global options to fill up this dialog down the line as we add more features to the spam filter :)

>If not, I think it would be useful to
> disable the show log button if the log file does not exist.

I think it's better UI to see the button get enabled when you enable logging instead of seeing it stay disabled and not understanding what action you have to take before you can click on the button. Even if that means brining up an empty log file. 
(Assignee)

Comment 19

12 years ago
(In reply to comment #15)
> (From update of attachment 218487 [details] [diff] [review] [edit])
> my one concern is about migration from the per-server prefs to the global
> prefs. What happens in that scenario?
> 

For the prefs I made global (manual mark mode and logging), I didn't try to
migrate account settings data. I didn't think it was a big deal for a user to
reset these values. Especially logging. 

I would think differently for some of the other settings
(like which junk folder to use, which address book), etc. were made global. Feel free to disagree. 

Comment 20

12 years ago
We should at least relnote it, but if it's not too hard, we could just migrate the pref from the default account, if it's not the default setting. Especially the manual mark mode, since that's going to be pretty visible if the user has overridden the default for any given account.

Comment 21

12 years ago
Will this fix or replace or at least help with Bugzilla Bug 323159
"Trust junk mail headers set by:" should be consistent in UI and log location/access?


Can we get some better documentation along with this? See Bugzilla Bug 328847
Junk mail needs documentation?


Thanks.

Comment 22

12 years ago
Comment on attachment 218487 [details] [diff] [review]
updated patch ready for review

>Index: mail/locales/en-US/chrome/messenger/prefs.properties
Would be nice if you could fix suite's version too, but see below ;-)

>+prefPanel-junk=Junk
I think this should say "Junk Settings" to match the panel.

>+ // if we made account changes to the spam settings, we'll need to re-initialize
>+ // our settings object
>+ if (server && server.spamSettings)
>+   server.spamSettings.initialize(server);
This fudge is really annoying me, but I've got some nsMsgIncomingServer cleanup that I really ought to attach to a bug before attacking this.

>+# ***** BEGIN LICENSE BLOCK *****
Would appreciate a JS-style licence, or at least /* */ around the whole block.

>+  // set up the whitelist UI
>+  var abList = document.getElementById("whiteListAbURI");
>+  menuitems = abList.getElementsByAttribute("id", document.getElementById('server.whiteListAbURI').value);
>+  abList.selectedItem = menuitems[0];
menulists have moved on since you first used them, I think... try this:
document.getElementById("whiteListAbURI").value = document.getElementById("server.whiteListAbURI").value);
Note: I tried to make the value apply directly to the menulist, which didn't work, for some reason.

>+  // set up trusted IP headers
>+  var serverFilterList = document.getElementById("useServerFilterList");
>+  var defaultServerItem = serverFilterList.getElementsByAttribute("value", 
>+                            document.getElementById('server.serverFilterName').value)[0];
>+  if (defaultServerItem)
>+    serverFilterList.selectedItem = defaultServerItem;
>+  else
>+   serverFilterList.selectedIndex = 0;
Again, this can be simplified using .value, e.g.
serverFilterList.value = document.getElementById("server.serverFilterName").value;
if (!serverFilterList.selectedItem)
  serverFilterList.selectedIndex = 0;

>+    if (entry.isFile())
>+    { 
>+      var fileName = entry.leafName;
>+      
>+      // we only care about files that end in .sfd 
>+      if (fileName.length > 4 && (fileName.lastIndexOf('.sfd') == fileName.length - 4))
>+      { 
>+        fileName = fileName.substring(0, fileName.length - 4);
I'd appreciate some more modern string matching too ;-)
if (entry.isFile() && /\.sfd$/.test(entry.leafName))
{
  var fileName = regexp.leftcontext;
  ...

>+        if (ispHeaderPopup.getElementsByAttribute("value", fileName)[0])
Prefer the use of .item(0) when you're unsure whether the item exists.

>+        var menuitem = document.createElement('menuitem');
>+        menuitem.setAttribute('label', fileName);
>+        menuitem.setAttribute('value', fileName);
>+        ispHeaderPopup.appendChild(menuitem);
With a modern menulist you can do
ispHeaderList.appendItem(fileName, fileName);
It will even create a menupopup for you!

>+# ***** BEGIN LICENSE BLOCK *****
Would appreciate an XML-style licence, or <!-- -->s


>+  <checkbox id="spamLevel" oncommand="updateSpamLevel();"
>+            accesskey="&level.accesskey;"  label="&level.label;"/>
No such function defined in your js.

>+      <column flex ="1"/>
Nit: unnecessary space before =. You might also consider checking your lines for trailing whitespace. (I don't know how many of the 43 suspicious lines were just moves).

>+      <row>
Your rows need align="center" otherwise wrapped checkboxes will look odd.

>+        <menulist id="whiteListAbURI" oncommand="onWhiteListAbURIChange();">
>+          <menupopup id="abPopup-menupopup" ref="moz-abdirectory://" 
>+                     datasources="rdf:addressdirectory"
>+                     sortActive="true"
>+                     sortDirection="ascending"
>+                     sortResource="http://home.netscape.com/NC-rdf#DirTreeNameSort">
>+            <template>
>+              <rule nc:IsWriteable="false"/>
>+              <rule nc:IsMailList="false">
>+                <menuitem uri="..."
I'd prefer the template on the menulist rather than on the menupopup i.e.
<menulist ... datasources=... >
  <template>
    <rule ... >
      <menupopup>
        <menuitem .../>

>+          <menupopup id="useServerFilter-menupopup">
>+          </menupopup>
If I hadn't already suggested removing it I would have suggested using <menupopup id="useServerFilter-menupopup"/> instead.

>+      <label id="purgeLabel" value="&purge2.label;"/>
It improves the visual effect if this also observes the move mode broadcaster.
Attachment #218487 - Flags: review?(neil) → review-
(Assignee)

Comment 23

12 years ago
Created attachment 220052 [details] [diff] [review]
migrate the new global settings

This extra patch migrates the values for:

manualMark, manualMarkMode and spamLogging

from the default account to the new global settings.

I forgot to include my change to mailnews.js which adds mail.spam.version with a value of 0.
Attachment #220052 - Flags: superreview?(bienvenu)

Comment 24

12 years ago
Comment on attachment 220052 [details] [diff] [review]
migrate the new global settings

great, thx.
Attachment #220052 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 25

12 years ago
Created attachment 220055 [details] [diff] [review]
updated patch with Neil's review comments

Neil,

I believe I addressed all of your review comments if you care to take another look the shared code again. Thanks!
Attachment #218487 - Attachment is obsolete: true
Attachment #220055 - Flags: review?(neil)

Comment 26

12 years ago
Comment on attachment 220055 [details] [diff] [review]
updated patch with Neil's review comments

>+  updateMoveTargetMode(document.getElementById('server.moveOnSpam').checked);
For some reason this isn't working when switching between sets of junk settings; the automatic deletion settings keep getting enabled. Off hand the only idea I can think of is to remove and reset the disabled attribute, but if that doesn't work then check it in anyway, I don't want another review ;-)

>+  document.getElementById('server.serverFilterName').value =
>+    document.getElementById("useServerFilterList").selectedItem.value;
>+  document.getElementById('server.whiteListAbURI').value =
>+    document.getElementById("whiteListAbURI").selectedItem.id;
Oops, I overlooked these... they should just say ").value;

>+  <grid>
>+    <columns>
>+      <column flex="1"/>
>+      <column/>
>+    </columns>
>+    
>+    <rows>
>+      <row>
>+        <checkbox id="server.useWhiteList" 
I was thinking that this row needs align="center" because the checkbox is in a flex="1" column ...

>+        <menulist id="whiteListAbURI" oncommand="onWhiteListAbURIChange();" datasources="rdf:addressdirectory"
>+                  ref="moz-abdirectory://" sortActive="true" sortDirection="ascending"
>+                  sortResource="http://home.netscape.com/NC-rdf#DirTreeNameSort">
>+            <template>
Even though it's a -w diff, I still think I can blame this 4-space indent and some miscellaneous trailing whitespace on you :-P

>+      <row>
>+        <checkbox id="server.useServerFilter" label="&ispHeaders.label;" accesskey="&ispHeaders.accesskey;"
... and so this row needs align="center" as well ...

>+        <grid class="specialFolderPickerGrid indent">
>+          <columns>
>+            <column/>
>+            <column flex="1"/>
>+          </columns>
>+          <rows>
>+            <row align="center">
>+              <radio id="moveTargetMode0" value="0" label="&junkFolderOn.label;" observes="broadcaster_moveMode"/>
>+              <menulist id="actionTargetAccount" observes="broadcaster_moveMode"
>+                        oncommand="onActionTargetChange(this, 'server.spamActionTargetAccount');" />
>+            </row>
>+            <row align="center">
>+              <radio id="moveTargetMode1" value="1" label="&otherFolder.label;" observes="broadcaster_moveMode"/>
... but these rows do not, because the radios aren't in a flex="1" column.

r=me with these nits fixed.
Attachment #220055 - Flags: review?(neil) → review+
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: blocking-thunderbird2+
Target Milestone: --- → Thunderbird2.0
(Assignee)

Comment 27

12 years ago
Created attachment 220151 [details] [diff] [review]
updated fix with review comments [checked in on the trunk and branch]

carrying forward david's sr and neil's r.
Attachment #220151 - Flags: superreview+
Attachment #220151 - Flags: review+
Attachment #220151 - Flags: approval-branch-1.8.1?
(Assignee)

Updated

12 years ago
Blocks: 335846
(Assignee)

Comment 28

12 years ago
Neil, in case you hadn't done so already, I filed Bug #335846 for the seamonkey folks to implement a global junk pref panel for spam logging, manualMark and manualMarkMode.

I'd like to get this landed by next week so we can start collecting feedback on it. Hopefully we can get the seamonkey panel ready by then. I'll check back in before I land.

Comment 29

12 years ago
Hmm, the am-junk.xul panel doesn't work correctly for me on Mac/PPC with current SM trunk builds (even though it's just a UI glitch):
I've two accounts (IMAP and Movemail), both with "Move new junk..." turned off, hence the radiobuttons and the checkbox below should be disabled. But when I select am-junk.xul of one account and then directly switch to am-junk.xul of the other account, the "Automatically..." checkbox and the following textbox stay active, while the final "days" gets disabled...
The attribute on the broadcaster gets set, it's just that the checkbox and textbox don't seem to care when their value isn't changed by the panel switch... 

Comment 30

12 years ago
Doh, I somehow skipped reading comment #26, especially the first paragraph... :(
So, where did the 
  [_] Mark junk as read
setting go? This should be part of the account settings junk panel, right?

Comment 31

12 years ago
(In reply to comment #30)
>So, where did the [_] Mark junk as read setting go?
Hmm, perhaps bug 193625 was never fixed for TB so mscott overlooked it?

Comment 32

12 years ago
> >So, where did the [_] Mark junk as read setting go?
> Hmm, perhaps bug 193625 was never fixed for TB so mscott overlooked it?

Probably, since the /mail/.../junkMail.xul doesn't contain it. I added that in my patch in 335846 for TB, too (touching am-junk.*), but it isn't actually in the scope of that bug...

(Assignee)

Comment 33

12 years ago
Created attachment 222535 [details] [diff] [review]
follow up fix for thunderbird to add mark junk as read to options panel. [checked in on the trunk and branch]
(Assignee)

Comment 34

12 years ago
Karsten, the main patch for this work has landed on the trunk and the branch so you should be good to go for the seamonkey changes in Bug 335846.
(Assignee)

Updated

12 years ago
Attachment #220151 - Flags: approval-branch-1.8.1? → approval-branch-1.8.1+
(Assignee)

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #220055 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #220052 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #220151 - Attachment description: updated fix with review comments → updated fix with review comments [checked in on the trunk and branch]
(Assignee)

Updated

12 years ago
Attachment #222535 - Attachment description: follow up fix for thunderbird to add mark junk as read to options panel → follow up fix for thunderbird to add mark junk as read to options panel. [checked in on the trunk and branch]

Comment 35

12 years ago
(In reply to comment #21)
> Will this fix or replace or at least help with Bugzilla Bug 323159
> "Trust junk mail headers set by:" should be consistent in UI and log
> location/access?
> 
> 
> Can we get some better documentation along with this? See Bugzilla Bug 328847
> Junk mail needs documentation?
> 
> 
> Thanks.

Just a reminder on the documentation of these changes. Thanks.
(Assignee)

Comment 36

12 years ago
Created attachment 222679 [details] [diff] [review]
[checked into the branch and trunk] additional change

When I added manual mark as read to the options UI yesterday, I forgot to change the spam settings code to make this a read only global option instead of an account specific option. 

This change does that and it removes mManualMark and mManualMarkMode variables which aren't used now that those options are global as well.

Note: I did not bump the interface ID for nsISpamSettings because it was changed with the rest of this checkin yesterday.
Attachment #222679 - Flags: superreview?(bienvenu)

Updated

12 years ago
Attachment #222679 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 37

12 years ago
(In reply to comment #36)
> Created an attachment (id=222679) [edit]
> additional change
> 
> When I added manual mark as read to the options UI yesterday, I forgot to
> change the spam settings code to make this a read only global option instead of
> an account specific option. 

I meant "mark spam as read" not manual mark.

Comment 38

12 years ago
Created attachment 222861 [details]
parts are not greyed out

There's something not correct with that new settings.

Checkbox "automatically delete junk mail older than" isn't greyed out if I start that dialog the first time.

If I check "Move new junk messages to:" and uncheck it again, checkbox "automatically delete junk mail older than" is greyed out now.

Seems to be a little cosmetic problem.
(Assignee)

Updated

12 years ago
Attachment #222679 - Attachment description: additional change → [checked into the branch and trunk] additional change
(Assignee)

Comment 39

12 years ago
(In reply to comment #38)
> There's something not correct with that new settings.
> 
> Checkbox "automatically delete junk mail older than" isn't greyed out if I
> start that dialog the first time.
> 


That's pretty weird. I haven't seen that. It's odd that the "days" label does get disabed but the preceding checkbox and text box are not disabled. Any errors in the JS console?

Comment 40

12 years ago
Looks good. Is there a similar bug to do this with message filters? Fixing that would help with Bugzilla Bug 323159
"Trust junk mail headers set by:" should be consistent in UI and log location/access

 What about the logs for these junk mail and filters? Can the log be accessed here as well? 

Updated

12 years ago
Blocks: 323159

Comment 41

12 years ago
(In reply to comment #39)
> (In reply to comment #38)
> > There's something not correct with that new settings.
> > 
> > Checkbox "automatically delete junk mail older than" isn't greyed out if I
> > start that dialog the first time.
> > 
> 
> That's pretty weird. I haven't seen that. It's odd that the "days" label does
> get disabed but the preceding checkbox and text box are not disabled. Any
> errors in the JS console?
> 

I can confirm this with no errors in JS - and a possible "Steps to Reproduce":
1. You gotta have 2 accounts to reproduce.
2. Uncheck both accounts' "Move new junk messages to".
3. Now open Account Settings again and visit account 1's Junk Settings and then account 2's. Account 2's "Automatically delete...." will be enable.

Comment 42

12 years ago
Where does one find the junk mail log now?

Comment 43

12 years ago
(In reply to comment #42)
> Where does one find the junk mail log now?

Tools > Options > Privacy > Junk

Comment 44

12 years ago
Is there a problem with junk filtering now? I see only 5 items there after weeks of not clearing it and lots of junk mail. I couldn't find an open bug specifically on this.
(Assignee)

Updated

12 years ago
Depends on: 339116
Blocks: 299826

Comment 45

12 years ago
(In reply to comment #44)
> Is there a problem with junk filtering now? I see only 5 items there after
> weeks of not clearing it and lots of junk mail. I couldn't find an open bug
> specifically on this.

That's possibly fallout from bug 339116.
No longer blocks: 299826
Depends on: 339275

Comment 46

12 years ago
I like the new Junk mail settings, but there are still some issues in my opinion.

Currently, there are 2 ways to detect junk mail in Thunderbird:
1. Internal bayesian filter
2. External server-side filters (eg SpamAssassin an SpamPal) through special headers
Both are independant and can be used individually or together. (BTW, I can't find any documentation on if and how the bayesian filter is influenced by the server-side filtering.) But this is not very obvious from the dialog. Maybe those two options should be arranged in a group (named "junk detection method" or something similar). Also, shouldn't the explanation about the training only refer to the bayesian filter? Finally, the rest of the options can go into a different group (eg "white-listing" and "handling").

Comment 47

11 years ago
Don't know if I need to open another bug for reporting only a typo:
pref-junk.dtd -> markAsReadOnSpam.acesskey
                                   ^
Please add the double "c", or the association between string and accesskey is lost... Typo is present both in trunk and branch...

Comment 48

11 years ago
(In reply to comment #47)
> Don't know if I need to open another bug for reporting only a typo:
> pref-junk.dtd -> markAsReadOnSpam.acesskey
>                                    ^
> Please add the double "c", or the association between string and accesskey is
> lost... Typo is present both in trunk and branch...
> 

Is this why junk mail settings isn't working right lately?

Comment 49

11 years ago
(In reply to comment #47)
> Don't know if I need to open another bug for reporting only a typo:
> pref-junk.dtd -> markAsReadOnSpam.acesskey

In Thunderbird, this file is 'privacy.dtd' (located in en-US.jar).  That file has the same typo; however, the key itself is active: Alt+K toggles the "Mark messages determined to be Junk as Read" checkbox.

There is no accesskey at all for the 'manualMark' checkbox nor its two radiobuttons.
(Assignee)

Comment 50

11 years ago
Created attachment 227425 [details] [diff] [review]
[fixed branch and trunk] fixes access key issues in the privacy options panel

Fixes the accesskey typo and adds missing access keys to the privacy panel.

Thanks mcow
Attachment #227425 - Flags: review?
(Assignee)

Updated

11 years ago
Attachment #227425 - Flags: review? → review?(bienvenu)

Updated

11 years ago
Attachment #227425 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

11 years ago
Attachment #227425 - Attachment description: fixes access key issues in the privacy options panel → [fixed branch and trunk] fixes access key issues in the privacy options panel

Comment 51

11 years ago
Can the log files be made easier to find (and purge)?

For reference, see Bugzilla Bug 323159
"Trust junk mail headers set by:" should be consistent in UI and log location/access 

Comment 52

11 years ago
*** Bug 219138 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Depends on: 351545

Comment 53

11 years ago
This bug should also depend on bug 328847.

Updated

11 years ago
Depends on: 355212
No longer blocks: 323159

Updated

10 years ago
Depends on: 342632
You need to log in before you can comment on or make changes to this bug.