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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kazhik, Assigned: jshin1987)
References
(Depends on 1 open bug)
Details
(Keywords: intl)
Attachments
(4 files)
900 bytes,
patch
|
Details | Diff | Splinter Review | |
1.84 KB,
patch
|
Details | Diff | Splinter Review | |
17.18 KB,
patch
|
Details | Diff | Splinter Review | |
1.55 KB,
patch
|
jshin1987
:
review+
Bienvenu
:
superreview+
asa
:
approval-aviary1.1a2+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
Can't make "Enabled" checkbox on/off if the filter name is in Japanese. Build: 2003041808-trunk/Linux, 2003041609-trunk/WinMe
Comment 2•21 years ago
|
||
This affects MacOSX as well and also seems to be triggered by any non-ASCII character in the filter name.
Comment 3•20 years ago
|
||
See also bug 242199 for more detailed description. These two bugs must be triggered by the same problem.
Comment 4•20 years ago
|
||
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•20 years ago
|
||
*** Bug 242199 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: MailNews → Core
Comment 6•20 years ago
|
||
*** Bug 252261 has been marked as a duplicate of this bug. ***
Comment 7•20 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•20 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•20 years ago
|
||
That analysis sounds quite plausible. I can try a patch...
Assignee: sspitzer → bienvenu
Comment 10•20 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•20 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•20 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•20 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•20 years ago
|
||
it just ends up calling nsRDFResource's GetValueUTF8
Comment 15•20 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•20 years ago
|
||
right, it won't compile - you just pass in uri, and it'll compile.
Comment 17•20 years ago
|
||
Comment 18•20 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•20 years ago
|
||
your fix didn't work for me, at least for my test case :-(
Comment 20•20 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?
Comment 21•20 years ago
|
||
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•20 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•20 years ago
|
||
*** Bug 273749 has been marked as a duplicate of this bug. ***
Comment 24•19 years ago
|
||
Similar problem is also seen if DBCS (i.e Chinese or Japanese) characters are included in filter names.
Comment 25•19 years ago
|
||
Convert the filter name to UTF8 before comparision
Assignee | ||
Comment 26•19 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•19 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•19 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•19 years ago
|
||
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•19 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•19 years ago
|
||
*** Bug 203783 has been marked as a duplicate of this bug. ***
Comment 31•19 years ago
|
||
David, I assume you can take care of porting this...
Attachment #185465 -
Flags: superreview?(bienvenu)
Attachment #185465 -
Flags: review?(jshin)
Updated•19 years ago
|
Attachment #185465 -
Flags: review?(jshin) → review?(jshin1987)
Updated•19 years ago
|
Attachment #185465 -
Flags: superreview?(bienvenu) → superreview+
Assignee | ||
Comment 32•19 years ago
|
||
Comment on attachment 185465 [details] [diff] [review] Suite ui patch r=jshin
Attachment #185465 -
Flags: review?(jshin1987) → review+
Updated•19 years ago
|
Attachment #185465 -
Flags: approval1.8b3?
Comment 33•19 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•19 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•19 years ago
|
||
thunderbird version checked in.
Comment 35•19 years ago
|
||
Fix checked in for the Suite.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 36•19 years ago
|
||
*** Bug 312296 has been marked as a duplicate of this bug. ***
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•