202729 - Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII (e.g Japanese or with German umlauts)

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Koike Kazuhiko]

Can't make "Enabled" checkbox on/off if the filter name is in Japanese.

Build: 2003041808-trunk/Linux, 2003041609-trunk/WinMe

Comment 1

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 2

[Göran Öberg]

This affects MacOSX as well and also seems to be triggered by any non-ASCII
character in the filter name.

Comment 3

[YAMAGUCHI Satoru]

See also bug 242199 for more detailed description.

These two bugs must be triggered by the same problem.

Comment 4

[WADA:World Anti-bad-Duping Agency]

I've got next JavaScript error (same as Thunderird bug 242199) when clicked
enable/disable button of a filer with Japanese Name
(20040309-trunk/Win2K+SP4)

Error: [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE) [nsIRDFResource.GetDelegate]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://messenger/content/FilterListDialog.js :: toggleFilter :: line 240" 
data: no]
Source File: chrome://messenger/content/FilterListDialog.js
Line: 240

Comment 5

[Mike Cowperthwaite]

*** Bug 242199 has been marked as a duplicate of this bug. ***

Comment 6

[Alexey Chernyak]

*** Bug 252261 has been marked as a duplicate of this bug. ***

Comment 7

[Clément VARALDI]

Bug still in the 1.0RC binary version of thunderbird for mac os X.

(it affects at least Linux, mac os ans Windows)

Comment 8

[Ahn Dal-Soo]

Here is what I found by browsing source code.

It seems the cause is:

/mozilla/source/mailnews/base/search/src/nsMsgFilterDelegateFactory.cpp

nsMsgFilterDelegateFactory::getFilterDelegate(nsIRDFResource *aOuter,
                                              nsIMsgFilter **aResult)
{
-snip-
    rv = aOuter->GetValue(getter_Copies(uri));
-snip-
}

And a resolution could be:
    rv = aOuter->GetValueUTF8(getter_Copies(uri));

Reason:
First I observed, by modifying toggleFilter() in FilterListDialog.js, the
content of aFilterURI, nsIRDFResource.Value, nsIRDFResource.ValueUTF8, and the
result of nsIRDFResource.EqualsString();

content/messenger/FilterListDialog.js

function toggleFilter(aFilterURI)
{
dump("aFilterURI = ");
dump(aFilterURI);
dump("\n");

var filterResource = gRDF.GetResource(aFilterURI);

dump("Value = ");
dump(filterResource.Value);
dump("\n");

dump("ValueUTF8 = ");
dump(filterResource.ValueUTF8);
dump("\n");

if (filterResource.EqualsString(aFilterURI))
	dump("Equal string returned true.\n");
else
	dump("Equal string returned false.\n");
-snip-
}

If filter name is ASCII, the contents of Value and ValueUTF8 are identical.
However they are different if filter name is Japanese. Here is the log of
dump(). For sake of English bugzilla, I substitute Japanese filter name with
English words just indicaing identity.

[ASCII]
aFilterURI = mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=test
Value = mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=test
ValueUTF8 = mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=test
EqualString returned true.

[Japanese]
aFilterURI =
mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=Japenese word A
Value = mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=Japanese
word A looking garbled
ValueUTF8 =
mailbox://pop.mail.yahoo.co.jp%3Aahndalsoo@localhost;filterName=Japanese word A
Equal string returned false.

toggleFilter() calls GetDelegate().

/mozilla/source/rdf/util/src/nsRDFResource.cpp

169 nsRDFResource::GetDelegate(const char* aKey, REFNSIID aIID, void** aResult)
170 {
-snip-
200     rv = delegateFactory->CreateDelegate(this, aKey, aIID, aResult);
-snip-
225 }

GetDelegate() calls CreateDelegate().

/mozilla/source/mailnews/base/search/src/nsMsgFilterDelegateFactory.cpp

nsMsgFilterDelegateFactory::CreateDelegate(nsIRDFResource *aOuter, const
char *aKey, const nsIID & aIID, void * *aResult)
 77 {
-snip-
 97         rv = getFilterDelegate(aOuter, getter_AddRefs(filter));
-snip-
115 }

CreateDelegate() calls getFilterDelegate().

136 nsMsgFilterDelegateFactory::getFilterDelegate(nsIRDFResource *aOuter,
137                                               nsIMsgFilter **aResult)
138 {
-snip-
142     rv = aOuter->GetValue(getter_Copies(uri));
-snip-
182 }

getFilterDelegate() calls nsRDFResource::GetValue() to get URI.

Though I haven't reviewed interface code between JavaScript and C++, I assume
the following correspondence.

nsRDFResource.Value in JavaScript = nsRDFResource.GetValue() in C++
nsRDFResource.ValueUTF8 in JavaScript = nsRDFResource.GetValueUTF8() in C++

The result of GetValue() and the content of aFilterURI don't match. Finlally
seaching the target URI fails and toggleFilter() gets NS_ERROR_FAILURE.

Therefore calling GetValueUTF8() instead of GetValue() should work out.

Comment 9

[David :Bienvenu]

That analysis sounds quite plausible. I can try a patch...

Comment 10

[David :Bienvenu]

GetValue and GetValueUTF8 do the same thing in nsRDFResource. The URI is
supposed to already be in utf8. So the problem is elsewhere. I believe we store
the filter name in utf8 already, in msgFilterRules.dat, but I wonder if we don't
mess that up at some point when reading it back in.

Comment 11

[Ahn Dal-Soo]

Yes, filter names are properly stored in msgFilterRules.dat. The URI is stored
in utf8 already when nsRDFResource is initialized.

I wonder how GetValueUTF8() and GetValue() can return different results if they
do the same thing in nsRDFResource. GetValueUTF8() just returns raw mURI member
variable but GetValue() cooks it by calling ToNewCString(). Here is the code:

128 NS_IMETHODIMP
129 nsRDFResource::GetValue(char* *aURI)
130 {
131     NS_ASSERTION(aURI, "Null out param.");
132     
133     *aURI = ToNewCString(mURI);
134 
135     if (!*aURI)
136         return NS_ERROR_OUT_OF_MEMORY;
137 
138     return NS_OK;
139 }
140 
141 NS_IMETHODIMP
142 nsRDFResource::GetValueUTF8(nsACString& aResult)
143 {
144     aResult = mURI;
145     return NS_OK;
146 }

I think ToNewCString() messes up non-ascii string. My point is that the value of
aFilterURI and the value searched in getFilterDelegate() must be identical.
Currently they are different. Simply calling GetValueUTF8() instead of
GetValue() should make them identical. Unfortunately I don't have build
environment for Thunderbird. If you give me a test build, I will try it with
pleasure.

I haven't figured out how ToNewCString() makes non-ascii string corrupted.

Comment 12

[David :Bienvenu]

I should have said that I tried calling GetValueUTF8 and it didn't work...so you
might be right that they return different values, but GetValueUTF8 didn't fix
the problem for me when I used some 8 bit chars...

Comment 13

[Ahn Dal-Soo]

Huh, it didn't work... Another possibility is that aFilterURI is corrupted. I
will have looks at this. Could you kindly provide your GetValueUTF8 version if
you can?

Comment 14

[David :Bienvenu]

it just ends up calling nsRDFResource's GetValueUTF8

Comment 15

[Ahn Dal-Soo]

I'm sorry. "rv = aOuter->GetValueUTF8(getter_Copies(uri));" doesn't work because
the type of argument is wrong.  Argument type is different between
GetValue(char* *aURI) and GetValueUTF8(nsACString& aResult). I will write my
version of a patch.

Comment 16

[David :Bienvenu]

right, it won't compile - you just pass in uri, and it'll compile.

Comment 17

[Ahn Dal-Soo]

Attachment: ValueUTF8

Comment 18

[Ahn Dal-Soo]

aFilterURI passed to toggleFilter() is already corrupted. The culprit is
res.Value in onFilterTreeKeyPress(). I don't understand why Value doesn't work
while ValueUTF8 works. This time I confirmed my fix works.

Comment 19

[David :Bienvenu]

your fix didn't work for me, at least for my test case :-(

Comment 20

[Ahn Dal-Soo]

Umm, it works just fine when I type in some Japanese letters for filter name and
click on the check box to turn on/off the filter. Could you give me your test
case if you could?

Comment 21

[WADA:World Anti-bad-Duping Agency]

Change summary because this bug is not Japanese filter name only problem.

Comment 22

[Jungshik Shin]

Let's keep Japanese because sometimes  'Japanese' as a keyword for bugzilla
query is pretty useful.

Comment 23

[Hiro]

*** Bug 273749 has been marked as a duplicate of this bug. ***

Comment 24

[Muni Sekhar Reddy]

Similar problem is also seen if DBCS (i.e Chinese or Japanese) characters are
included in filter names.

Comment 25

[Venkateswara Rao]

Attachment: Convert the filter name to UTF8 before comparision

Convert the filter name to UTF8 before comparision

Comment 26

[Jungshik Shin]

(In reply to comment #25)
> Created an attachment (id=181508) [edit]
> Convert the filter name to UTF8 before comparision
> 
> Convert the filter name to UTF8 before comparision

Oh my gosh. We should track down the root cause. The following code is suspicious:

http://lxr.mozilla.org/seamonkey/source/mailnews/base/util/nsMsgIncomingServer.cpp#1911

I'll take a further look. 

Comment 27

[Jungshik Shin]

filter-related code and nsMsgIncoming.cpp need some serious i18n-love. bug
291679 was filed for that.

Comment 28

[Jungshik Shin]

Let me take it. I made a patch yesterday, but haven't been able to test it
because somehow my build on Linux keeps failing to start (not related with this
bug). 

Comment 29

[Jungshik Shin]

Attachment: patch

I thought this would work, but I got unexpected errors when I opened 'Filter
management' window as shown below. I'm trying to figure this out.

WARNING: GetTarget(): unknown filter delegate!
, file e:/works/moz/mozilla/mailnews/base/search/src/nsMsgFilterDataSource.cpp,

line 206
WARNING: GetTarget(): unknown filter delegate!
, file e:/works/moz/mozilla/mailnews/base/search/src/nsMsgFilterDataSource.cpp,

line 206
--DOMWINDOW == 7
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
[n
sIRDFResource.GetDelegate]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)" 
location
: "JS frame :: chrome://messenger/content/FilterListDialog.js :: toggleFilter
::
 line 256"  data: no]
************************************************************
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
[n
sIRDFResource.GetDelegate]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)" 
location
: "JS frame :: chrome://messenger/content/FilterListDialog.js :: toggleFilter
::
 line 256"  data: no]
************************************************************
JavaScript error: chrome://messenger/content/FilterListDialog.js, line 510:
filt
er has no properties

Comment 30

[Karsten Düsterloh]

*** Bug 203783 has been marked as a duplicate of this bug. ***

Comment 31

[[e-mail]]

Attachment: Suite ui patch

David, I assume you can take care of porting this...

Comment 32

[Jungshik Shin]

Comment on attachment 185465 [details] [diff] [review]
Suite ui patch

r=jshin

Comment 33

[David :Bienvenu]

Comment on attachment 185465 [details] [diff] [review]
Suite ui patch

the same fix applies to thunderbird.

Comment 34

[David :Bienvenu]

thunderbird version checked in.

Comment 35

[[e-mail]]

Fix checked in for the Suite.

Comment 36

[Ray Booysen]

*** Bug 312296 has been marked as a duplicate of this bug. ***