Closed Bug 202729 Opened 21 years ago Closed 19 years ago

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

Categories

(MailNews Core :: Filters, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kazhik, Assigned: jshin1987)

References

(Depends on 1 open bug)

Details

(Keywords: intl)

Attachments

(4 files)

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

Build: 2003041808-trunk/Linux, 2003041609-trunk/WinMe
mass re-assign.
Assignee: naving → sspitzer
This affects MacOSX as well and also seems to be triggered by any non-ASCII
character in the filter name.
See also bug 242199 for more detailed description.

These two bugs must be triggered by the same problem.
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
*** Bug 242199 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
*** Bug 252261 has been marked as a duplicate of this bug. ***
Bug still in the 1.0RC binary version of thunderbird for mac os X.

(it affects at least Linux, mac os ans Windows)
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.
That analysis sounds quite plausible. I can try a patch...
Assignee: sspitzer → 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.
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.
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...
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?
it just ends up calling nsRDFResource's GetValueUTF8
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.
right, it won't compile - you just pass in uri, and it'll compile.
Keywords: intl
Attached patch ValueUTF8Splinter Review
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.
your fix didn't work for me, at least for my test case :-(
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?
Change summary because this bug is not Japanese filter name only problem.
Summary: Can't make "Enabled" checkbox on/off if the filter name is in Japanese → Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII
Let's keep Japanese because sometimes  'Japanese' as a keyword for bugzilla
query is pretty useful.
Summary: Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII → Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII (e.g Japanese)
*** Bug 273749 has been marked as a duplicate of this bug. ***
Similar problem is also seen if DBCS (i.e Chinese or Japanese) characters are
included in filter names.
Convert the filter name to UTF8 before comparision
(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. 
filter-related code and nsMsgIncoming.cpp need some serious i18n-love. bug
291679 was filed for that.
Depends on: 291679
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). 
Assignee: bienvenu → jshin1987
Attached patch patchSplinter Review
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
Summary: Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII (e.g Japanese) → Can't make "Enabled" checkbox on/off if the filter name is in non-ASCII (e.g Japanese or with German umlauts)
*** Bug 203783 has been marked as a duplicate of this bug. ***
Attached patch Suite ui patchSplinter Review
David, I assume you can take care of porting this...
Attachment #185465 - Flags: superreview?(bienvenu)
Attachment #185465 - Flags: review?(jshin)
Attachment #185465 - Flags: review?(jshin) → review?(jshin1987)
Attachment #185465 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 185465 [details] [diff] [review]
Suite ui patch

r=jshin
Attachment #185465 - Flags: review?(jshin1987) → review+
Attachment #185465 - Flags: approval1.8b3?
Comment on attachment 185465 [details] [diff] [review]
Suite ui patch

the same fix applies to thunderbird.
Attachment #185465 - Flags: approval-aviary1.1a2?
Attachment #185465 - Flags: approval1.8b3?
Attachment #185465 - Flags: approval1.8b3+
Attachment #185465 - Flags: approval-aviary1.1a2?
Attachment #185465 - Flags: approval-aviary1.1a2+
thunderbird version checked in.
Fix checked in for the Suite.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
*** Bug 312296 has been marked as a duplicate of this bug. ***
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: