Am I missing something, or is this a bug in nsLocalMailFolder.cpp?

Kent James

unread,

Nov 1, 2006, 1:43:59 PM11/1/06

to

I don't understand the copying of "keywords" to "junkscoreorigin" in the
following recent addition to nsLocalMailFolder.cpp. Is this a bug, if
not can someone please explain why the keywords are overwriting the
junkscoreorigin? Shouldn't the third SetStringProperty be to "keywords"?

Existing code:

void nsMsgLocalMailFolder::CopyPropertiesToMsgHdr(nsIMsgDBHdr *destHdr,
nsIMsgDBHdr *srcHdr)
{
nsXPIDLCString sourceString;
srcHdr->GetStringProperty("junkscore", getter_Copies(sourceString));
destHdr->SetStringProperty("junkscore", sourceString);
srcHdr->GetStringProperty("junkscoreorigin",
getter_Copies(sourceString));
destHdr->SetStringProperty("junkscoreorigin", sourceString);
srcHdr->GetStringProperty("keywords", getter_Copies(sourceString));
destHdr->SetStringProperty("junkscoreorigin", sourceString);

nsMsgLabelValue label = 0;
srcHdr->GetLabel(&label);
destHdr->SetLabel(label);
}

David Bienvenu

unread,

Nov 2, 2006, 11:19:46 AM11/2/06

to

Thx for noticing, that's a bug. I'll fix it now.

- David

Mike Cowperthwaite

unread,

Nov 2, 2006, 1:36:39 PM11/2/06

to

David Bienvenu wrote:
> Thx for noticing, that's a bug. I'll fix it now.
>
> - David
>
> Kent James wrote:
>> I don't understand the copying of "keywords" to "junkscoreorigin" in
>> the following recent addition to nsLocalMailFolder.cpp. Is this a
>> bug, if not can someone please explain why the keywords are
>> overwriting the junkscoreorigin? Shouldn't the third SetStringProperty
>> be to "keywords"?

Is there a user-visible symptom to this problem?

David, did you open a bug for that?

David Bienvenu

unread,

Nov 2, 2006, 2:18:18 PM11/2/06

to

Mike Cowperthwaite wrote:
>>> I don't understand the copying of "keywords" to "junkscoreorigin" in
>>> the following recent addition to nsLocalMailFolder.cpp. Is this a
>>> bug, if not can someone please explain why the keywords are
>>> overwriting the junkscoreorigin? Shouldn't the third
>>> SetStringProperty be to "keywords"?
>
> Is there a user-visible symptom to this problem?

I think there probably is one, but it's not the obvious one of
maintaining tags when copying local messages between folders.

>
> David, did you open a bug for that?

I'm going to try to find an existing bug about tags not being maintained
when copying messages. While debugging this, I discovered that we
weren't maintaining keywords when copying local messages to an imap
server that supports user-defined keywords. So I'll probably combine the
fixes in one bug.

- David

Kent James

unread,

Nov 2, 2006, 3:13:26 PM11/2/06

to

Here's another minor bug you might want to add to that. In
nsImapMailFolder.cpp near line 6876:

if (mDatabase && msgDBHdr)
{
nsMsgLabelValue label;
nsXPIDLCString junkScore, junkScoreOrigin;
nsMsgPriorityValue priority;
msgDBHdr->GetStringProperty("junkscore",
getter_Copies(junkScore));
msgDBHdr->GetStringProperty("junkscoreorigin",
getter_Copies(junkScoreOrigin));
if (!junkScore.IsEmpty()) // ignore already scored messages.
mDatabase->SetAttributesOnPendingHdr(msgDBHdr, "junkscore",
junkScore.get(), 0);
if (!junkScoreOrigin.IsEmpty())
mDatabase->SetAttributesOnPendingHdr(msgDBHdr,
"junkscoreorigin", junkScore.get(), 0);

In that last line, the junkScore.get() should be junkScoreOrigin.get()

- Kent

David Bienvenu

unread,

Nov 2, 2006, 4:27:13 PM11/2/06

to

Kent James wrote:
>
> Here's another minor bug you might want to add to that. In
> nsImapMailFolder.cpp near line 6876:
>
> if (mDatabase && msgDBHdr)
> {
> nsMsgLabelValue label;
> nsXPIDLCString junkScore, junkScoreOrigin;
> nsMsgPriorityValue priority;
> msgDBHdr->GetStringProperty("junkscore",
> getter_Copies(junkScore));
> msgDBHdr->GetStringProperty("junkscoreorigin",
> getter_Copies(junkScoreOrigin));
> if (!junkScore.IsEmpty()) // ignore already scored messages.
> mDatabase->SetAttributesOnPendingHdr(msgDBHdr,
> "junkscore", junkScore.get(), 0);
> if (!junkScoreOrigin.IsEmpty())
> mDatabase->SetAttributesOnPendingHdr(msgDBHdr,
> "junkscoreorigin", junkScore.get(), 0);
>
> In that last line, the junkScore.get() should be junkScoreOrigin.get()
>
> - Kent

Thanks, I'll fix that too. I guess we don't use the junkscore origin much!

- David

Kent James

unread,

Nov 2, 2006, 9:44:09 PM11/2/06

to

David Bienvenu wrote:

> Thanks, I'll fix that too. I guess we don't use the junkscore origin much!
>
> - David

I'm running an extension now that uses a custom column to show the
junkscoreorigin. I've also modified the code to add junkscoreorigins of
"whitelist" (which is a hard GOOD)and "filter". With these changes,
junkscoreorigin becomes much more interesting as a diagnostic of spam
processing for particular messages.