Created attachment 171813 [details] [diff] [review] Patch I also changed some other things in this pane: - Corrected some accesskeys ("r" was used two times, "I" was used for Inline). - Removed some very old commented-out code and an useless comment. I couldn't find a hint why this was commented out instead of deleted. It is just a copy of the XUL that is used to generate the Character Encoding menulist. - Removed the unused string languagesTitle.label. - Changed the forward setting from a menulist to a radiogroup. This way this option is less hidden and one needs less mouse-clicks to change it. - Changed "Character Encoding" to "Default Character Encoding" for consistency with the pref panes Navigator|Languages and Mail & Newsgroups|Message Display. Also the related checkbox below this menulist speaks about "Always use this default ...". If this is too much unrelated stuff in this patch, please protest. ;-) Alternatively I could file a separate bug.
Comment on attachment 171813 [details] [diff] [review] Patch OK, a quick glance through the patch: > <hbox align="center"> >+ <label value="&forwardMsg.label;"/> >+ <radiogroup id="forwardMessageMode" orient="horizontal" >+ prefstring="mail.forward_message_mode"> >+ <radio value="2" label="&inline.label;" accesskey="&inline.accesskey;"/> >+ <radio value="0" label="&asAttachment.label;" accesskey="&asAttachment.accesskey;"/> >+ </radiogroup> > </hbox> If you put the label inside the (centred) radiogroup you shouldn't need the hbox any more. >-<!ENTITY forwardMsg.accesskey "F"> You could give this key to a more deserving field. >-<!ENTITY inline.accesskey "I"> >+<!ENTITY inline.accesskey "e"> I don't like this change but only because the Message/Forward As menu also uses _I_nline >+<!ENTITY autoSave.label "Automatically save the message every"> >+<!ENTITY autoSave.accesskey "m"> >+<!ENTITY autoSaveEnd.label "minutes"> This needs an l10n note. >-<!ENTITY sendDefaultCharset.accesskey "r"> >-<!ENTITY replyInDefaultCharset.accesskey "r"> You don't have to remove both "r" access keys!
Created attachment 171820 [details] [diff] [review] Patch V1.1 Addressed Neil's initial comments.
Wrong patch again?
Created attachment 171825 [details] [diff] [review] Patch V1.1 (for real) I uploaded the wrong patch... I guess I better go to bed now. ;-)
> - Changed the forward setting from a menulist to a radiogroup. > This way this option is less hidden and one needs less mouse-clicks > to change it. You think it's a good idea to raise the prominence of this pref? I wouldn't, for political reasons. I personally (as a recipient) much prefer the default (attachment), because inline removes headers, forces a layout and removes the information that it's an attached mail, which means you can't drag&drop the forwarded mail directly into your folder (if Mailnews allowed that) etc.. Maybe move that into another bug. Otherwise: Thanks for the patch.
Nevermind my last comment
Comment on attachment 171825 [details] [diff] [review] Patch V1.1 (for real) Well, the panel's a little too "busy" for my liking, but I can't obviously see how to improve it. By the way, populateFonts() belongs in Startup().
Fix checked in.