439548 - When tag name is substring of other tag, tag can not be added.

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[WADA:World Anti-bad-Duping Agency]

Spin-off of Bug 433175 Commant #15 (trunk part).

With Name=Thunderbird,Version=3.0a2pre,BuildID=2008061203 on MS Win-XP SP2.

When tag name is substring of other tag, tag can not be added.
(1) Define tags named "abc", "ab", "bc".
(2) Copy two mails in a new folder.
(3-1) Mail-1: add tag, "ab", "bc", then "abc" => all tags are added.
(3-2) Mail-2: add tag of "abc".
      Once "abc" is added, unable to add "ab" or "bc". (Tb does do nothing) 
Problem of (3-2) doesn't occur with Tb 2.0.0.14. 

Note:
Although Tb trunk currently can not generate "X-Mozilla-Keys: abc ab bc" due to this bug, Bug 433175 (spin in Virtul Folder) probably occurs on Tb trunk too, if such tag combination was generated in Tb 2 era.

Comment 1

[Karsten Düsterloh]

JFTR: This bug exists in current SeaMonkey trunk also.

Comment 2

[Karsten Düsterloh]

Attachment: fix MsgFindKeywords

The problem is MsgFindKeywords in nsMsgUtils.cpp, which has two problems:
1. It encounters "ab" at position 0 in "abc", finds out it doesn't match and runs on - but it doesn't check if it's still inside the string and runs through memory until it finds somthing remotely matching.
2. But this is only a side effect of a string function mismatch: keywords.Find ends up in the obsolete "nsTString_CharT::Find( const nsCString& aString, PRBool aIgnoreCase=PR_FALSE, PRInt32 aOffset=0, PRInt32 aCount=-1 ) const;" instead of the intended "nsAString::Find(const self_type& aStr, PRUint32 aOffset, ComparatorFunc c = DefaultComparator) const;", and so the parameters are met *wrongly*!

This patch takes some complexity out of the function and adds the required parameter, so that it works - but actually the function mismatch should be fixed, because we do want to use the frozen API!

David, ideas?

Comment 3

[David :Bienvenu]

Karsten, thx for working on this - I was just starting to debug it. I'll test your fix. You're right that we should try to fix this using the frozen api but I don't know what that actually entails. I bet Neil knows though :-)

Comment 4

[David :Bienvenu]

Comment on attachment 325452 [details] [diff] [review]
fix MsgFindKeywords

+  // 'keyword' is the single keyword to be sought
+  // 'keywords' is a space delimited list of keywords to be searched,
+  // which may be deteriorated to a single keyword or even be empty

instead of "to be sought", maybe "we're looking for"

instead of "deteriorated to a", how about "just".

This patch works a lot better than what was there before. I'm hitting this assertion:
          NS_ASSERTION(keywords.IsEmpty() || keywords.CharAt(0) != ' ', "space only keyword");

when I do a remove all keywords but it may be that an other bug messed up my keyword headers. I'll try to fix that other bug and see if I still see the assertion.

Comment 5

[WADA:World Anti-bad-Duping Agency]

FYI. Bug 367011 exists in "0 Remove All Tags". 

Comment 6

[WADA:World Anti-bad-Duping Agency]

FYI 2. Another one for "0 Remove All Tags" : Bug 368210.

Comment 7

[[e-mail]]

(In reply to comment #2)
>but actually the function mismatch should be
>fixed, because we do want to use the frozen API!
Yeah, well blame bsmedberg for making the internal and external APIs different.

Comment 8

[David :Bienvenu]

Comment on attachment 325452 [details] [diff] [review]
fix MsgFindKeywords

We should get this in.

Could you use #ifdef MOZILLA_INTERNAL_API ...#else ... to make this work when we switch to frozen linkages?

Comment 9

[Karsten Düsterloh]

Will do, as soon as cvs.mozilla.org likes me again. :-(

Comment 10

[Karsten Düsterloh]

Attachment: final version [checked in]

Landed this on CVS trunk (1.9.0).

Comment 11

[WADA:World Anti-bad-Duping Agency]

VERIFIED with Name=Thunderbird,Version=3.0a2pre,BuildID=2008062003 on Win-XP SP2.

FYI.
All of "abc", "ab", "bc" is found by Virtual folder, and loop when "add tag, then define Virtual Folder" (Bug 439835) has disappeared.
But loop when "define Virtual Folder, then add tag" (Bug 433175) was still observed. (i.e. Now we can re-produce Bug 433175 with Tb Trunk.)