Closed Bug 556786 Opened 10 years ago Closed 10 years ago

Filter junk status "Not Junk" won't save

Categories

(MailNews Core :: Filters, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.1b2

People

(Reporter: rkent, Assigned: rkent)

Details

(Keywords: regression)

Attachments

(1 file)

STR:

Setup a new filter with filter action "Junk Status" is "Not Junk". Exit saving the filter, and restart. The filter will have "Junk" instead of "Not Junk" as the status.

Came across this trying to fix bug 540385, regression from a string-related change.
I'm going to remove this from my patch for bug 540385, and land it later. Here is the chunk:

diff --git a/mailnews/base/search/src/nsMsgFilterList.cpp b/mailnews/base/search/src/nsMsgFilterList.cpp
--- a/mailnews/base/search/src/nsMsgFilterList.cpp
+++ b/mailnews/base/search/src/nsMsgFilterList.cpp
@@ -687,17 +687,17 @@ nsresult nsMsgFilterList::LoadTextFilter
             currentFilterAction->SetType(nsMsgFilterAction::AddTag);
             currentFilterAction->SetStrValue(keyword);
           }
         }
         else if (type == nsMsgFilterAction::JunkScore)
         {
           nsresult res;
           PRInt32 junkScore = value.ToInteger(&res, 10);
-          if (NS_FAILED(res))
+          if (NS_SUCCEEDED(res))
             currentFilterAction->SetJunkScore(junkScore);
         }
         else if (type == nsMsgFilterAction::Forward ||
                  type == nsMsgFilterAction::Reply ||
                  type == nsMsgFilterAction::AddTag ||
                  type == nsMsgFilterAction::Custom)
         {
           currentFilterAction->SetStrValue(value);
Brief history of this bug: the original error was actually introduced in 2008 in bug 395701 - but it only affected cases where MOZILLA_INTERNAL_API was not set, which meant no normal uses.

Recently, the 2010-03-03 checkin of bug 377319 removed the MOZILLA_INTERNAL_API check, and exposed the problem introduced by the previous bug. I think it is worth noting that there are lots of places in the code where previous dual internal/external string code existed, but the external code was virtually untested. So this may not be the only regression introduced by recent removals of MOZILLA_INTERNAL_API.
Attachment #436994 - Flags: superreview?(neil)
Attachment #436994 - Flags: review?(neil)
Whiteboard: [needs r/sr Neil]
Comment on attachment 436994 [details] [diff] [review]
NS_FAILED -> NS_SUCCEEDED

Stealing review as this is simple and I'd like to see it fixed.

I actually found the STR were to set the junk status to "Junk" and then it would be "Not Junk" after a restart. Maybe we'd started from slightly different setups.

Still, with the patch it works nicely. r/sr=Standard8.
Attachment #436994 - Flags: superreview?(neil)
Attachment #436994 - Flags: superreview+
Attachment #436994 - Flags: review?(neil)
Attachment #436994 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [needs r/sr Neil]
Target Milestone: --- → Thunderbird 3.1b2
You need to log in before you can comment on or make changes to this bug.