Closed
Bug 110127
Opened 23 years ago
Closed 23 years ago
Cache charset for nsMsgI18NGetDefaultMailCharset
Categories
(MailNews Core :: Internationalization, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.7
People
(Reporter: nhottanscp, Assigned: nhottanscp)
Details
Attachments
(2 files, 3 obsolete files)
3.94 KB,
patch
|
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
4.91 KB,
patch
|
Details | Diff | Splinter Review |
The function gets a mail compose default charset from pref.
It can be cached and hook up notification for changes instead of read the value
everytime. We also want to change the function name to say explicitely this is
for compose default instead of view default, e.g.
nsMsgI18NGetDefaultMailCompseCharset.
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.7
Assignee | ||
Comment 1•23 years ago
|
||
Currently there are three places which call the function. I think two of them
(search and eudora) use it incorrectly. What they need is view_default not
send_default.
/mailnews/base/search/src/nsMsgSearchAdapter.cpp, line 326
/mailnews/compose/src/nsMsgCompFields.cpp, line 82
/mailnews/import/eudora/src/nsEudoraCompose.cpp, line 623
My current plan is to remove nsMsgI18NGetDefaultMailCharset and add a function
to get send_default to nsIMsgCompose. By adding to nsIMsgCompose, .js files
which reads send_default from pref also take advantage of the cached value.
For Eudora, I think it just need to read view_default from the pref no cache is
needed.
For search, cache view_default in the search class.
cc to ducarroz
Assignee | ||
Comment 2•23 years ago
|
||
Assignee | ||
Comment 3•23 years ago
|
||
Attachment #58165 -
Attachment is obsolete: true
Comment 4•23 years ago
|
||
Comment on attachment 58169 [details] [diff] [review]
For search, cache default charset to the class. For eudora, changed to get a pref directly.
r=naving
looks ok to me, but could
you explain why we need
srcCharset and dstCharset
in SearchAdapter.
Attachment #58169 -
Flags: review+
Assignee | ||
Comment 5•23 years ago
|
||
For local search, UI charset is unique (from the pref default) but the target
charset may vary based on the folder charset (the user can assign different
charset per folder), so need to have two charsets.
Assignee | ||
Comment 6•23 years ago
|
||
Comment 7•23 years ago
|
||
Comment on attachment 58404 [details] [diff] [review]
Cache default charset in nsMsgCompFields, removed nsMsgI18NGetDefaultMailCharset.
Looks good. R=ducarroz
Attachment #58404 -
Flags: review+
Comment 8•23 years ago
|
||
I prefer using IsEmpty() for the following code with nsXPIDLCString:
+ if (charset)
+ m_DefaultCharacterSet.AssignWithConversion(charset);
+ else
+ m_DefaultCharacterSet.Assign("ISO-8859-1");
so if (charset.IsEmpty())
m_DefaultCharacterSet.Assign("ISO-8859-1");
else
m_DefaultCharacterSet.AssignWithConversion(charset);
Other than that, looks OK.
Assignee | ||
Comment 9•23 years ago
|
||
Attachment #58404 -
Attachment is obsolete: true
Comment 10•23 years ago
|
||
Comment on attachment 59964 [details] [diff] [review]
Revised 58404 with bienvenu's comment.
sr=bienvenu
Attachment #59964 -
Flags: superreview+
Comment 11•23 years ago
|
||
+ if (!m_defaultCharset)
change it to if (m_defaultCharset.IsEmpty()) and sr=bienvenu
Assignee | ||
Comment 12•23 years ago
|
||
Attachment #58169 -
Attachment is obsolete: true
Comment on attachment 59964 [details] [diff] [review]
Revised 58404 with bienvenu's comment.
I checked in a fix for the OS/2 bustage following the checkin of this patch by
changing |inline nsresult| to |NS_IMETHODIMP| for
nsMsgCompFields::GetDefaultCharacterSet, which can't be inlined anyway.
Assignee | ||
Comment 14•23 years ago
|
||
Right, they cannot be inlined, thanks for the fix.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: MailNews → Core
Updated•17 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•