Closed
Bug 301291
Opened 19 years ago
Closed 17 years ago
Forward-inline ignores outgoing-charset preference
Categories
(MailNews Core :: Composition, defect)
MailNews Core
Composition
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9beta1
People
(Reporter: elharo, Assigned: petr.hroudny)
References
Details
(Keywords: verified1.8.1.12)
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
Bienvenu
:
review+
mscott
:
superreview+
dveditz
:
approval1.8.1.12-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
petr.hroudny
:
review+
dveditz
:
approval1.8.1.12+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Frequently when I'm replying to a message I am warned that the message contains
characters that cannot be sent in the current encoding and asking if I want to
send in UTF-8. I always click yes.
Thunderbird should not even ask me. It should just send in UTF-8. It should
probably never send in anything else.
Reproducible: Couldn't Reproduce
Steps to Reproduce:
It happens frequently, but I can;t make it happen on command. It probably
depends on the message I'm replaying to.
Actual Results:
Thunderbird asks me if I want to send in UTF-8.
Expected Results:
Just send in UTF-8 without bothering me.
In my opinion unless htere is some deep SMTP issue I'm unaware of, one of the
following should be done (in decreasing order of preference)
1. Send everything in UTF-8 all the time without exception. Never ask the user.
Do not allow them to configure anything else.
2. When necessary, send in UTF-8 without asking the user.
3. Put a checkbox in that dialog allowing the user to "Never show this alert
again" or some such. i.e. make the preference sticky.
4. Provide some hidden preference somewhere to make automatic UTF-8 the default.
If it exists already it's too hidden. I couldn't find it.
Comment 1•19 years ago
|
||
And what about email clients that don't support UTF-8 ?
Reporter | ||
Comment 2•19 years ago
|
||
Which ones exactly? It's 2005. At this point anyone using a client that doesn't
support UTF-8 needs to upgrade; but I think pretty much all the clients do
support UTF-8.
In any case, do you really expect any user to know whether the recipient on the
other end is using a client that supports UTF-8 or not?
Comment 3•19 years ago
|
||
(In reply to comment #2)
> Which ones exactly? It's 2005. At this point anyone using a client that doesn't
> support UTF-8 needs to upgrade; but I think pretty much all the clients do
> support UTF-8.
>
> In any case, do you really expect any user to know whether the recipient on the
> other end is using a client that supports UTF-8 or not?
see bug 194862 and bug 249530
Comment 4•19 years ago
|
||
Thunderbird already has a menu to select the character encoding, IMO this should
just default to UTF-8 and be made "sticky", so that if you regularly need to
communicate with people in some legacy encoding you can select that once and
have it be the default in the future.
See also bug 224391.
Comment 5•19 years ago
|
||
(In reply to comment #0)
> Thunderbird should not even ask me. It should just send in UTF-8. It should
> probably never send in anything else.
There are two ways to make this happen:
1) Set your default outgoing preference to UTF-8 *and* check the box for
"Use the default encoding in replies."
-OR-
2) Set up the fallback preferences for UTF-8. As an example, I have this:
intl.fallbackCharsetList.ISO-8859-1 set to "UTF-8"
You need to create a fallback list for every character set you might use for
composing mail; and the string value for the preference can be a comma-separated
list of different encodings.
(In reply to comment #4)
> [TB] already has a menu to select the character encoding, IMO this should
> just default to UTF-8 and be made "sticky"
The preference for outgoing encoding *is* sticky, and if you enforce that for
replies as well (see (1) above), you get exactly what you ask for.
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 6•19 years ago
|
||
First, I do have Thunderbird configured as you suggest, wiuth UTF-8 for outgoing
messages and "Use the default encoding in replies." and yet twice just yesterday
I still saw the dialog asking me if I wanted to send the reply in UTF-8. This
happens a few times a week. So there seems to be an out and out bug somewhere,
and at least on Mac OS X and Thunderbird 1.1 alpha 2. The preference is not
being respected.
Second, I still think this shouldn't even be a preference. It should just send
in UTF-8 and not bother the user.
For some more UTF-8 everywhere propaganda see
http://www.google.com/url?sa=t&ct=res&cd=1&url=http%3A//www.ibm.com/developerworks/xml/library/x-utf8/
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
Comment 7•19 years ago
|
||
(In reply to comment #6)
> First, I do have Thunderbird configured as you suggest, wiuth UTF-8 for
> outgoing messages and "Use the default encoding in replies." and yet twice
> just yesterday I still saw the dialog asking me if I wanted to send the reply
> in UTF-8. This happens a few times a week. So there seems to be an out and out
> bug somewhere, and at least on Mac OS X and Thunderbird 1.1 alpha 2. The
> preference is not being respected.
I agree, that is a bug. The next time you get this, hit Cancel and go back to
the compose window; check under Options to see which encoding was in fact set.
And, check the encoding on the message being replied to. Report back here.
Also check if it's reproducible -- does hitting Reply again from the same
message result in the same encoding being set on the Options dialog? Or,
possibly, is the encoding set as expected to UTF-8 *until* you click Send?
If you find a message where this is reproducible, it would be helpful if you
could provide it; save it as a .EML file and attach to this bug (using the
Create New Attachment link above) -- or, if it's sensitive, you could forward it
(as an attachment) directly to me.
Reporter | ||
Comment 8•19 years ago
|
||
OK. Just happenend again. This is now in 1.5 beta 1, still on Mac OS X.
Under options I see that character encoding is to Western/.ISO-8859-1. Note that
I did check and "Always use UTF-8" and "Use default encoding in replies" is
checked in my preferences.
However, this time it did not happen when replying. It happened when forwarding
a message inline. Is there some reason forwarding doesn't pick up the default
reply encoding? I suspect it should.
Comment 9•19 years ago
|
||
(In reply to comment #8)
> However, this time it did not happen when replying. It happened when
> forwarding a message inline.
I'd bet folding money you've actually seen this only with forward-inline.
> Is there some reason forwarding doesn't pick up the default
> reply encoding? I suspect it should.
Forward Inline behaves much as Edit As New, which means the message is
initialized according to the original message, mostly. (The most visible
exception is that Forward Inline of an HTML message is still set up as a plain
message, if that's how the sending identity (From:) is configured, and vice
versa.)
If instead the character set for forward-inline was always the one you
specified, but in the opposite situation -- you were configured to always send
ISO-8859-1 but you inline-forwarded a message containing Unicode characters --
you'd still be getting this prompt.
Still, I guess it would make sense for Forward-Inline to adhere to the same pref
as Reply: if "use default encoding in replies" is checked, that encoding should
also be used for forwarding.
I can't find a dupe, so confirming.
Reproduced with TB 1.6a1-1003, Seamonkey 1.0a-0910, Win2K: moving to Core,
trunk, updating platform/OS.
See also bug 287517.
Status: UNCONFIRMED → NEW
Component: Message Compose Window → MailNews: Composition
Ever confirmed: true
OS: MacOS X → All
Product: Thunderbird → Core
Hardware: Macintosh → All
Summary: Always confirms UTF-8 send → Forward-inline ignores outgoing-charset preference
Version: unspecified → Trunk
Assignee | ||
Comment 11•17 years ago
|
||
Looked at mailnews/compose/src/nsMsgCompose.cpp
The code snippet providing the default charset for replies is quite
strightforward:
// use send_default_charset if reply_in_default_charset is on.
nsCOMPtr<nsIPrefBranch> prefs (do_GetService(NS_PREFSERVICE_CONTRACTID));
if (prefs)
{
PRBool replyInDefault = PR_FALSE;
prefs->GetBoolPref("mailnews.reply_in_default_charset",
&replyInDefault);
if (replyInDefault) {
nsXPIDLString str;
NS_GetLocalizedUnicharPreferenceWithDefault(prefs, "mailnews.send_defa
ult_charset",
EmptyString(), str);
if (!str.IsEmpty())
LossyCopyUTF16toASCII(str, charset);
}
}
I think this bug can be resolved by putting similar code into function which handles inline-forwarding.
Updated•17 years ago
|
Assignee: mscott → nobody
QA Contact: composition
Assignee | ||
Comment 12•17 years ago
|
||
Looked further at source - for Inline forwards, the function
exits 70 lines sooner:
// If we are forwarding inline, mime did already setup the compose fields
therefore we should stop now
if (type == nsIMsgCompType::ForwardInline )
return rv;
Could someone please advice if this is the correct place for the check if
"mailnews.reply_in_default_charset" is set?
Comment 13•17 years ago
|
||
Petr: looks like it could be the correct place, please attach a patch for it if it works.
Assignee | ||
Comment 14•17 years ago
|
||
Please check this patch.
Forward inline now works as expected (tested with TB 2.0.0.6)
Attachment #282376 -
Flags: superreview?
Attachment #282376 -
Flags: review?
Attachment #282376 -
Flags: approval1.9?
Attachment #282376 -
Flags: approval1.8.1.9?
Attachment #282376 -
Flags: approval1.8.1.8?
Attachment #282376 -
Flags: approval1.8.0.14?
Attachment #282376 -
Flags: approval-thunderbird3?
Attachment #282376 -
Flags: approval-thunderbird2?
Comment 15•17 years ago
|
||
Comment on attachment 282376 [details] [diff] [review]
Patch against TB 2.0.0.6
Thanks for putting together a patch!
However, it needs to be updated to trunk as the code is undergoing significant string cleanup.
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgCompose.cpp#1749
After that, to get the code reviewed (and super reviewed) set the r? and sr? flags to the email of a suitable reviewer, like bienvenu and/or mscott (look at cvs history).
The other flags aren't needed, at least at this point.
Not strictly necessary, but it's easier to apply patches if you diff from the source root (stand in the 'mozilla' dir when doing cvs diff).
Attachment #282376 -
Flags: superreview?
Attachment #282376 -
Flags: review?
Attachment #282376 -
Flags: approval1.9?
Attachment #282376 -
Flags: approval1.8.1.9?
Attachment #282376 -
Flags: approval1.8.1.8?
Attachment #282376 -
Flags: approval1.8.0.14?
Attachment #282376 -
Flags: approval-thunderbird3?
Attachment #282376 -
Flags: approval-thunderbird2?
Assignee | ||
Updated•17 years ago
|
Attachment #282376 -
Attachment description: Patch to ensure forward-inline honours outgoing-charset → Patch against TB 2.0.0.6
Assignee | ||
Comment 16•17 years ago
|
||
Updated patch against CVT trunk.
Attachment #282513 -
Flags: superreview?
Attachment #282513 -
Flags: review?(bienvenu)
Comment 17•17 years ago
|
||
Petr, thx for the patch. A couple comments - please use consistent braces style. This:
+ if (replyInDefault) {
+ nsString str;
should be
+ if (replyInDefault)
+ {
+ nsString str;
+ nsCString charset;
+ NS_GetLocalizedUnicharPreferenceWithDefault(prefs, "mailnews.send_default_charset",
+ EmptyString(), str);
I suspect you left out a { here: Either that or the indentation is wrong.
+ if (!str.IsEmpty())
+ LossyCopyUTF16toASCII(str, charset);
+ m_compFields->SetCharacterSet(charset.get());
+ }
+ }
could you submit a new patch with those two things addressed, and re-request review from me? Otherwise, the patch looks OK to me...
Assignee | ||
Comment 18•17 years ago
|
||
Thanks for the comments - applied.
Attachment #282376 -
Attachment is obsolete: true
Attachment #282513 -
Attachment is obsolete: true
Attachment #282732 -
Flags: superreview?
Attachment #282732 -
Flags: review?(bienvenu)
Attachment #282513 -
Flags: superreview?
Attachment #282513 -
Flags: review?(bienvenu)
Comment 19•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
thx, Petr.
Attachment #282732 -
Flags: superreview?(mscott)
Attachment #282732 -
Flags: superreview?
Attachment #282732 -
Flags: review?(bienvenu)
Attachment #282732 -
Flags: review+
Comment 20•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
thanks for the patch!
Attachment #282732 -
Flags: superreview?(mscott) → superreview+
Assignee | ||
Comment 21•17 years ago
|
||
Thanks for reviewing! Could someone with CVS write access please check it in?
I'd also like to have this included in TB 2.0.0.x, but when I look into
TB 2.0.0.6 sources, there are some differences regarding strings.
For 2.0.0.x the declarations in my patch should be modified to:
nsXPIDLString str;
nsXPIDLCString charset;
and also the final charset assignment should read:
m_compFields->SetCharacterSet(charset);
Thanks.
Updated•17 years ago
|
Assignee: nobody → petr.hroudny
Target Milestone: --- → mozilla1.9 M9
Comment 22•17 years ago
|
||
I'm afraid only security and stability fixes go into tb2.0, so you'll have to wait for tb3.
Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v <-- nsMsgCompose.cpp
new revision: 1.531; previous revision: 1.530
done
->FIXED
Status: NEW → RESOLVED
Closed: 19 years ago → 17 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 23•17 years ago
|
||
Well, TB3 is several months ahead AFAIK...
In our setup we need to ensure that all emails leave encoded in UTF-8 and this bug means inline-forwards leak in different charsets.
Any chance to make an exception and include it into TB2 ?
Thanks.
Target Milestone: mozilla1.9 M9 → Future
Comment 24•17 years ago
|
||
Erh... I've seen a strange crash in thunderbird since this patch was added.
If I want to reply to a news article with a "à é è" or other non-english letters, composition window is opened, and a second letter, thunderbird is crashing !
Could it be related ?
I am using thunderbird under linux.
Assignee | ||
Comment 25•17 years ago
|
||
Code from my patch is only used when you *forward* message to someone else, and
only when your preferences are set to forward inline. It does not change anything
for replies:
if (type == nsIMsgCompType::ForwardInline )
....
Comment 26•17 years ago
|
||
Ok. Sorry for the spam. I will look at other nsMsgCompose.cpp patch. Sorry if I upset you :(
Comment 27•17 years ago
|
||
When a bug is fixed we set the target milestone to the nearest official release the fix will be in, so moz1.9 M9 in this case.
> Any chance to make an exception and include it into TB2 ?
You are free to set the approval1.8.1.8/9? flag of the patch, but generally these kind of changes aren't approved for inclusion.
Target Milestone: Future → mozilla1.9 M9
Assignee | ||
Comment 28•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
I'd like to kindly ask for inclusion into next Thunderbird 2.0 rebubild.
Please see comment #23 for motivation and #21 for 2.0 specifics. Thanks
Attachment #282732 -
Flags: approval1.8.1.9?
Attachment #282732 -
Flags: approval1.8.1.8?
Comment 29•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
Will have to wait for next release.
Attachment #282732 -
Flags: approval1.8.1.8?
Comment 30•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
next TB is 1.8.1.11
Attachment #282732 -
Flags: approval1.8.1.10? → approval1.8.1.11?
Comment 31•17 years ago
|
||
Comment on attachment 282732 [details] [diff] [review]
Patch v2
minusing this non-branch patch. We'd need a patch that applies if you're going to get someone else to checkin. But not sure this is something we'd take even then.
Attachment #282732 -
Flags: approval1.8.1.12? → approval1.8.1.12-
Assignee | ||
Comment 32•17 years ago
|
||
This is the patch for 1.8 branch. Taking review+ from the original patch.
Attachment #297175 -
Flags: review+
Attachment #297175 -
Flags: approval1.8.1.12?
Attachment #297175 -
Flags: approval1.8.0.15?
Attachment #297175 -
Flags: approval1.8.0.14?
Assignee | ||
Updated•17 years ago
|
Attachment #297175 -
Flags: approval1.8.0.15?
Attachment #297175 -
Flags: approval1.8.0.14?
Comment 33•17 years ago
|
||
Comment on attachment 297175 [details] [diff] [review]
Patch for 1.8 branch
approved for 1.8.1.12, a=dveditz for release-drivers
Attachment #297175 -
Flags: approval1.8.1.12? → approval1.8.1.12-
Assignee | ||
Comment 34•17 years ago
|
||
(In reply to comment #33)
> (From update of attachment 297175 [details] [diff] [review])
> approved for 1.8.1.12, a=dveditz for release-drivers
Thanks for the approval, could someone please check it into CVS?
Updated•17 years ago
|
Attachment #297175 -
Flags: approval1.8.1.12- → approval1.8.1.12+
Comment 35•17 years ago
|
||
Phil said he could land this on Sunday at the earliest, or Reed can sooner.
Keywords: checkin-needed
Whiteboard: [needs check on the 1.8 branch]
Comment 36•17 years ago
|
||
MOZILLA_1_8_BRANCH
Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v <-- nsMsgCompose.cpp
new revision: 1.460.2.35; previous revision: 1.460.2.34
done
Keywords: checkin-needed → fixed1.8.1.12
Whiteboard: [needs check on the 1.8 branch]
Assignee | ||
Comment 37•17 years ago
|
||
works as expected [Thunderbird 2.0.0.12pre (20080120) nightly], thanks to all involved in fixing this.
Keywords: fixed1.8.1.12 → verified1.8.1.12
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
•