Last Comment Bug 847172 - sort entries in the "Trust junk mail headers set by" menulist
: sort entries in the "Trust junk mail headers set by" menulist
Status: RESOLVED FIXED
: polish
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: Thunderbird 22.0
Assigned To: :aceman
:
Mentors:
Depends on: 846694 946669
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-03 06:50 PST by :aceman
Modified: 2013-12-10 12:28 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.38 KB, patch)
2013-03-03 07:38 PST, :aceman
no flags Details | Diff | Review
patch v2 (3.41 KB, patch)
2013-03-03 08:21 PST, :aceman
iann_bugzilla: review+
rkent: review+
Details | Diff | Review
patch v3 (3.42 KB, patch)
2013-03-27 12:48 PDT, :aceman
acelists: review+
Details | Diff | Review

Description :aceman 2013-03-03 06:50:15 PST
Sort the entries (spam filter names) in the "Trust junk mail headers set by" menulist. Currently they are random, as the directory service returns them. On the same machine I get different orders with different TB versions.
Comment 1 :aceman 2013-03-03 07:38:13 PST
Created attachment 720416 [details] [diff] [review]
patch

Apply on top of patch in bug 846694.
Comment 2 Sebastian H. [:aryx][:archaeopteryx] 2013-03-03 08:11:58 PST
Comment on attachment 720416 [details] [diff] [review]
patch

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

::: mailnews/base/prefs/content/am-junk.js
@@ +257,5 @@
>      if (ispDirectory)
> +      menuEntries.push.apply(menuEntries, buildServerFilterListFromDir(ispDirectory, menuEntries));
> +  }
> +
> +  menuEntries.sort();

Array.sort() is case sensitive:
["Aaron", "bethany", "Camille"].sort() returns ["Aaron, "Camille", "bethany"]
Two possibilities to fix this:
https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIXULSortService and http://mxr.mozilla.org/comm-central/source/editor/ui/dialogs/content/EdDialogCommon.js#1005
Comment 3 :aceman 2013-03-03 08:21:03 PST
Created attachment 720426 [details] [diff] [review]
patch v2

Thanks, so I'll use .localeCompare.
Comment 4 Ian Neal 2013-03-17 15:24:17 PDT
Comment on attachment 720426 [details] [diff] [review]
patch v2

Seems to do what it says on the tin r=me
Comment 5 Kent James (:rkent) 2013-03-26 12:12:26 PDT
Comment on attachment 720426 [details] [diff] [review]
patch v2

Looks good.
Comment 6 Ryan VanderMeulen [:RyanVM] 2013-03-27 07:34:41 PDT
This doesn't apply cleanly. Please rebase.
Comment 7 :aceman 2013-03-27 12:48:34 PDT
Created attachment 730290 [details] [diff] [review]
patch v3
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-03-27 13:16:26 PDT
https://hg.mozilla.org/comm-central/rev/99523d1da295

Note You need to log in before you can comment on or make changes to this bug.