Closed Bug 112904 Opened 23 years ago Closed 23 years ago

MimeInlineText's charset need to be initialized before setting font

Categories

(MailNews Core :: Internationalization, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: shanjian, Assigned: shanjian)

References

Details

Attachments

(1 file, 1 obsolete file)

This bug was found by Jungshik when he was working with bug 102623. 
He wrote:
There's a very strange problem unrelated with my
patch that caused my patch not to work.  
In mimemoz2.cpp, charset is always set to "us-ascii"

1937 nsresult GetMailNewsFont(MimeObject *obj, PRBool styleFixed, char
*fontName, PRUint32 nameBuffSize, 
1938 PRInt32 *fontPixelSize, PRInt32 *fontSizePercentage)
1939 {
1940 nsresult rv = NS_OK;
1941 1942 nsIPref *aPrefs = GetPrefServiceManager(obj->options);
1943 if (aPrefs) {
1944 MimeInlineText *text = (MimeInlineText *) obj;
1945 nsCAutoString aCharset;
1946 PRUnichar *unicode = nsnull;
1947 nsCAutoString convertedStr;
1948 nsCAutoString variableFontType;   // serif, sans-serif
1949 
1950 // get a charset
1951 if (!text->charset || !(*text->charset))
1952 aCharset.Assign("us-ascii");
1953 else
1954 aCharset.Assign(text->charset);

It used to work fine, but somehow text->charset
is NULL for SC, J and K messages and later langGroup
is set to x-western because charset is 'us-ascii'.

Therefore, to see my code work, I have to hardcode
charset to 'gb2312', 'iso-2022-jp', and 'euc-kr'
in place of 'us-ascii'.
This bug was introduced with one of my mime charset change. I have to 
delay the initialization of charset after MimeInlineText_initialize
since some information is not available at that time (namely 'option').

Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
ducarroz, seth,
Could you give r/sr? thanks.
Blocks: 102623
No longer blocks: 102623
Shanjian,
Thanks again for the fix. 
As for your patch, how about adding if-statement like this:
 if (!text->initializeCharset)
in mimemoz2.cpp and moving it after 
'MimeInlineText  *text = (MimeInlineText *) obj;'
Just thought that may be safer (no difference right now,
but just in case in the future initialize_charset() is
called earlier in another place).

my 0.02 :-)

Attached patch updated patchSplinter Review
Attachment #59894 - Attachment is obsolete: true
Jungshik, 
You are right. Thought it is not a problem at this time, it is a good 
idea to put the if statement there. Thanks for correcting me that. 
I updated my patch.
Comment on attachment 59946 [details] [diff] [review]
updated patch

R=ducarroz
Attachment #59946 - Flags: review+
Blocks: 102623
Comment on attachment 59946 [details] [diff] [review]
updated patch

sr=sspitzer
Attachment #59946 - Flags: superreview+
fix checked in. 
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Shanjian, could you help verifying this? Thanks a lot.
QA Contact: ji → shanjian
verified in debugger.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
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: