Comment 18 for bug 584632

Revision history for this message
In , Mscott-mozilla (mscott-mozilla) wrote :

Comment on attachment 260269
Updated patch that addresses TT special case

some minor nits:

+ nsXPIDLString font_face;

font_face should be a nsString

This:

+ if(NS_LITERAL_STRING("tt").Equals(font_face))

should be
font_face.EqualsLiteral("tt")

and does it need to be a case insensitive compare?

Can this:

aText.Append(NS_LITERAL_STRING("</TT>"));

be aText.AppendLiteral(...)?

Use a nsString here:
+ nsXPIDLString font_size;

could this be a simple switch statement?

+ if(NS_LITERAL_STRING("x-small").Equals(font_size))
+ font_size.Assign(NS_LITERAL_STRING("-2"));
+ else if(NS_LITERAL_STRING("small").Equals(font_size))
+ font_size.Assign(NS_LITERAL_STRING("-1"));
+ else if(NS_LITERAL_STRING("large").Equals(font_size))
+ font_size.Assign(NS_LITERAL_STRING("+1"));
+ else if(NS_LITERAL_STRING("x-large").Equals(font_size))
+ font_size.Assign(NS_LITERAL_STRING("+2"));
+ else if(NS_LITERAL_STRING("xx-large").Equals(font_size))
+ font_size.Assign(NS_LITERAL_STRING("+3"));
+ else
+ font_size.Assign(NS_LITERAL_STRING(""));

if not, the comparison statements should be of the form:

if (font_size.EqualsLiteral("xx-large"))

* NS_GetLocalizedUnicharPreferenceWithDefault(prefBranch, "msgcompose.font_size", NS_LITERAL_STRING(""), font_size);

why don't we just read the pref directly from the pref service instead of using this localized unichar preference routine. I don't think this pref is a localized pref.

I haven't looked at the mime part yet, I'll try to get to that soon! thanks for taking a look at this.