The default bug view has changed. See this FAQ.

"Trust junk mail headers set by" dropdown shows only a number 5

RESOLVED FIXED in Thunderbird 22.0

Status

MailNews Core
Account Manager
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

({regression})

Trunk
Thunderbird 22.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird21+ fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
"Trust junk mail headers set by" dropdown in account manager/ junk settings shows only a number 5 instead of the names of the server classifiers supported.

I think this regressed by bug 824150:
    14.2 +++ b/mailnews/base/prefs/content/am-junk.js
    14.3 @@ -272,21 +272,21 @@ function buildServerFilterListFromDir(aD
    14.4    // now iterate over each file in the directory looking for .sfd files
    14.5    let entries = aDir.directoryEntries
    14.6                      .QueryInterface(Components.interfaces.nsIDirectoryEnumerator);
    14.7  
    14.8    while (entries.hasMoreElements())
    14.9    {
   14.10      let entry = entries.nextFile;
   14.11      // we only care about files that end in .sfd
   14.12 -    if (entry.isFile() && /\.sfd$/.test(entry.leafName))
   14.13 +    if (entry.isFile() && entry.leafName.endsWith(".sfd"))
   14.14      {
   14.15        let fileName = RegExp.leftContext;
   14.16        // if we've already added an item with this name, then don't add it again.
   14.17 -      if (!aISPHeaderList.getElementsByAttribute("value", fileName).item(0))
   14.18 +      if (!aISPHeaderList.querySelector('[value="'+ fileName + '"]'))
   14.19          aISPHeaderList.appendItem(fileName, fileName);
   14.20      }
   14.21    }
   14.22  }

As the /regular expression/ was removed, it looks like the RegExp object(?) now returns something else as expected. This is easy to fix by converting that line to string operations on entry.leafName.
I'll do it shortly.
(Assignee)

Comment 1

4 years ago
The offending patch http://hg.mozilla.org/comm-central/rev/89fa8b687178 landed on Feb 04, so this will be needed for branch 21.
tracking-thunderbird21: --- → ?
Created attachment 719913 [details] [diff] [review]
patch to fix the issue

Sorry for the breakage.
Attachment #719913 - Flags: review?(mconley)
(Assignee)

Comment 3

4 years ago
I wanted to convert the ".sfd" to a constant so that the .endWith and -4 could be derived from it. In case it changes in the future.

Even with the patch, I am not sure the pref mail.server.%serverkey%.serverFilterName is saved properly (I did not find it in prefs.js), I'd like to check that out.
Attachment #719913 - Flags: review?(mconley)
(Assignee)

Comment 4

4 years ago
Created attachment 720405 [details] [diff] [review]
patch v2

Ok, it looks like the mail.server.serverX.serverFilterName pref is not set if the value matches mail.server.default.serverFilterName . But that seems fine, as retrieving the
Attachment #719913 - Attachment is obsolete: true
Attachment #720405 - Flags: review?(mconley)
(Assignee)

Comment 5

4 years ago
...retrieving the pref value returns the default value as if it was the value of the pref.
(Assignee)

Updated

4 years ago
Blocks: 847172
(Assignee)

Updated

4 years ago
Attachment #720405 - Flags: review?(iann_bugzilla)

Updated

4 years ago
Attachment #720405 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 720405 [details] [diff] [review]
patch v2

Review of attachment 720405 [details] [diff] [review]:
-----------------------------------------------------------------

Just one nit - beyond that, I'm happy with it.

::: mailnews/base/prefs/content/am-junk.js
@@ +269,5 @@
>   */
>  function buildServerFilterListFromDir(aDir, aISPHeaderList)
>  {
> +  // Now iterate over each file in the directory looking for .sfd files.
> +  const suffix = ".sfd";

consts should begin with k's, so this should be const kSuffix.
Attachment #720405 - Flags: review?(mconley) → review+
(Assignee)

Comment 7

4 years ago
Created attachment 725940 [details] [diff] [review]
patch v3

OK, thanks.
Attachment #720405 - Attachment is obsolete: true
Attachment #725940 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/81e97e309a34
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 22.0
(Assignee)

Comment 9

4 years ago
Comment on attachment 725940 [details] [diff] [review]
patch v3

[Approval Request Comment]
Regression caused by (bug #): bug 824150
User impact if declined: server junk filtering gets broken/unable to be set when the user enters the Junk settings pane.
Testing completed (on c-c, etc.): Verified fix on Daily 20130320030725.
Risk to taking this patch (and alternatives if risky): probably can't be much worse than it is.
Attachment #725940 - Flags: approval-comm-aurora?
Attachment #725940 - Flags: approval-comm-aurora? → approval-comm-aurora+
tracking-thunderbird21: ? → +
Checked in: https://hg.mozilla.org/releases/comm-aurora/rev/19386d8aa499
status-thunderbird21: --- → fixed
You need to log in before you can comment on or make changes to this bug.