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

RESOLVED FIXED

Status

MailNews Core
Filters
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: Koike Kazuhiko, Assigned: Jungshik Shin)

Tracking

(Depends on: 1 bug, {intl})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

15 years ago
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

Comment 2

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

Comment 3

14 years ago
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

Comment 5

14 years ago
*** Bug 242199 has been marked as a duplicate of this bug. ***
Product: MailNews → Core

Comment 6

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

Comment 7

13 years ago
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

13 years ago
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

13 years ago
That analysis sounds quite plausible. I can try a patch...
Assignee: sspitzer → bienvenu

Comment 10

13 years ago
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

13 years ago
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

13 years ago
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

13 years ago
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

13 years ago
it just ends up calling nsRDFResource's GetValueUTF8

Comment 15

13 years ago
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

13 years ago
right, it won't compile - you just pass in uri, and it'll compile.
(Assignee)

Updated

13 years ago
Keywords: intl

Comment 17

13 years ago
Created attachment 168509 [details] [diff] [review]
ValueUTF8

Comment 18

13 years ago
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

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

Comment 20

13 years ago
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
(Assignee)

Comment 22

13 years ago
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)

Comment 23

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

Comment 24

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

Comment 25

13 years ago
Created attachment 181508 [details] [diff] [review]
Convert the filter name to UTF8 before comparision

Convert the filter name to UTF8 before comparision
(Assignee)

Comment 26

13 years ago
(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. 
(Assignee)

Comment 27

13 years ago
filter-related code and nsMsgIncoming.cpp need some serious i18n-love. bug
291679 was filed for that.
Depends on: 291679
(Assignee)

Comment 28

13 years ago
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
(Assignee)

Comment 29

13 years ago
Created attachment 181976 [details] [diff] [review]
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

Updated

13 years ago
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)

Comment 30

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

Comment 31

13 years ago
Created attachment 185465 [details] [diff] [review]
Suite ui patch

David, I assume you can take care of porting this...
Attachment #185465 - Flags: superreview?(bienvenu)
Attachment #185465 - Flags: review?(jshin)

Updated

13 years ago
Attachment #185465 - Flags: review?(jshin) → review?(jshin1987)

Updated

13 years ago
Attachment #185465 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 32

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

r=jshin
Attachment #185465 - Flags: review?(jshin1987) → review+

Updated

13 years ago
Attachment #185465 - Flags: approval1.8b3?

Comment 33

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

the same fix applies to thunderbird.
Attachment #185465 - Flags: approval-aviary1.1a2?

Updated

13 years ago
Attachment #185465 - Flags: approval1.8b3?
Attachment #185465 - Flags: approval1.8b3+
Attachment #185465 - Flags: approval-aviary1.1a2?
Attachment #185465 - Flags: approval-aviary1.1a2+

Comment 34

13 years ago
thunderbird version checked in.

Comment 35

13 years ago
Fix checked in for the Suite.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 36

12 years ago
*** 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.