Last Comment Bug 410333 - get rid of the send in utf-8 question-dialog, just silently switch to utf-8 if necessary
: get rid of the send in utf-8 question-dialog, just silently switch to utf-8 i...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: mozilla1.9beta5
Assigned To: Magnus Melin
:
Mentors:
: 172061 427525 (view as bug list)
Depends on:
Blocks: 328938
  Show dependency treegraph
 
Reported: 2007-12-31 07:40 PST by Magnus Melin
Modified: 2009-12-25 04:33 PST (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (25.83 KB, patch)
2007-12-31 07:40 PST, Magnus Melin
mnyromyr: review+
mozilla: superreview+
Details | Diff | Splinter Review
proposed fix (wording change for replyInDefaultCharset) (5.28 KB, patch)
2008-03-26 12:48 PDT, Magnus Melin
philringnalda: review+
neil: superreview+
Details | Diff | Splinter Review
screenshot thunderbird (48.70 KB, image/png)
2008-03-26 12:50 PDT, Magnus Melin
no flags Details
screenshot seamonkey (66.60 KB, image/png)
2008-03-26 12:52 PDT, Magnus Melin
no flags Details

Description Magnus Melin 2007-12-31 07:40:37 PST
Created attachment 294962 [details] [diff] [review]
proposed fix

Per Karsten ~ bug 224391 comment 18.

This patch gets rid of the "Send in UTF-8" dialog, and silently switches to utf-8 when necessary. It's protecting users from making unnecessary choices most of them not understand. 

I didn't really see why we should let anyone choose to send broken mails anyway.
Comment 1 Karsten Düsterloh 2008-01-01 10:31:29 PST
Comment on attachment 294962 [details] [diff] [review]
proposed fix

Many thanks for taking this, Magnus!
Comment 2 Magnus Melin 2008-01-09 11:46:53 PST
Will also resolve at least bug 328938 and bug 311821.
Comment 3 rsx11m 2008-03-07 09:12:14 PST
> This patch gets rid of the "Send in UTF-8" dialog, and silently switches to
> utf-8 when necessary. It's protecting users from making unnecessary choices
> most of them not understand. 

Agreed, that dialog is more confusing than helpful. My only concern is that the patch apparently does not honor the mailnews.reply_in_default_charset setting. Thus, if replying to a message that contains characters which cannot be encoded in the default character set, the presence of those characters would prompt the message to be sent in UTF-8 despite the user preference of always applying the selected default character encoding.

Maybe treat as "sendInUTF8" for mailnews.reply_in_default_charset=false but as "sendAnyway" if mailnews.reply_in_default_charset=true?
Comment 4 Magnus Melin 2008-03-07 10:44:47 PST
I don't see how that's related, this would only kick in if your reply contains characters the selected charset can't handle, the selected charset being set by the pref or not.
Comment 5 rsx11m 2008-03-07 10:59:44 PST
Unless I'm missing something, it should kick in if you are replying to a message which is in a different character encoding than your own default. Quoting that message, when it contains characters not representable in your default encoding, would prompt the upgrade to UTF-8, correct? In that case, the patch's behavior would override the user's preference to always reply in his or her own encoding. While for the reasons you mention it is questionable to send out a message with broken characters, that preference should be honored if set by the user.
Comment 6 Magnus Melin 2008-03-07 11:32:42 PST
(In reply to comment #5)
> Unless I'm missing something, it should kick in if you are replying to a
> message which is in a different character encoding than your own default.

Only if they can't coexist, so only for some cases. "Always reply in my default encoding" would instead become "always reply in my default encoding when possible", which I assume is what people expect. Why would anyone contentiously send broken messages (in 2008, when it's perfectly acceptable to assume utf-8 support)? There is no point supporting a pref just for the sake of it.
Comment 7 rsx11m 2008-03-07 11:47:12 PST
Ok, I see your point. My understanding was that this preference means to send the user's encoding "no matter what"; given your interpretation, it would simply mean to start off with the user's preferred encoding rather than the actual encoding of the message replied to, then move to UTF-8 if a character doesn't fit.

Concern resolved.
Comment 8 Petr Hroudný 2008-03-09 00:02:38 PST
This bug is actually trying to avoid broken messages. If someone sets default charset to iso-8859-1, it would be unwise to enforce it ultimately and completely disallow e.g. latin-2 characters completely.

So yes, it should work as you describe - if reply_in_default_charset is set, Thunderbird should start off with your default charset (and not with the charset from original message). But if the text doesn't fit it should silently switch to UTF-8 like in all other cases without asking the users questions they don't understand and without giving them opportunity to break emails by clicking "Send anyway".


Comment 9 rsx11m 2008-03-09 07:13:53 PDT
Thanks, makes perfectly sense now. I was reading that preference too literally.
Comment 10 Daira Hopwood 2008-03-20 19:36:43 PDT
I agree with comment #6, but in that case the UI text for that pref should be
changed to "always reply in my default encoding when possible" (preferably in all localizations), rather than being left as it is.
Comment 11 Magnus Melin 2008-03-24 11:29:12 PDT
David: any concerns with this, or is it just waiting in the sr queue?
Comment 12 David :Bienvenu 2008-03-24 11:48:20 PDT
Comment on attachment 294962 [details] [diff] [review]
proposed fix

no, I agree that killing this dialog would be a good thing, and is worth trying out.
Comment 13 Magnus Melin 2008-03-26 09:47:25 PDT
Checking in mail/components/compose/content/MsgComposeCommands.js;
/cvsroot/mozilla/mail/components/compose/content/MsgComposeCommands.js,v  <--  MsgComposeCommands.js
new revision: 1.127; previous revision: 1.126
done
Checking in mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties,v  <--  composeMsgs.properties
new revision: 1.17; previous revision: 1.16
done
Checking in mailnews/compose/resources/content/MsgComposeCommands.js;
/cvsroot/mozilla/mailnews/compose/resources/content/MsgComposeCommands.js,v  <--  MsgComposeCommands.js
new revision: 1.413; previous revision: 1.412
done
Checking in mailnews/compose/src/nsComposeStrings.h;
/cvsroot/mozilla/mailnews/compose/src/nsComposeStrings.h,v  <--  nsComposeStrings.h
new revision: 1.7; previous revision: 1.6
done
Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v  <--  nsMsgCompose.cpp
new revision: 1.558; previous revision: 1.557
done
Checking in mailnews/compose/src/nsMsgPrompts.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgPrompts.cpp,v  <--  nsMsgPrompts.cpp
new revision: 1.32; previous revision: 1.31
done
Checking in mailnews/compose/src/nsMsgPrompts.h;
/cvsroot/mozilla/mailnews/compose/src/nsMsgPrompts.h,v  <--  nsMsgPrompts.h
new revision: 1.9; previous revision: 1.8
done
Checking in mailnews/compose/src/nsMsgSend.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgSend.cpp,v  <--  nsMsgSend.cpp
new revision: 1.419; previous revision: 1.418
done
Checking in mailnews/compose/src/nsMsgSendReport.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgSendReport.cpp,v  <--  nsMsgSendReport.cpp
new revision: 1.17; previous revision: 1.16
done
Comment 14 Magnus Melin 2008-03-26 12:38:09 PDT
Missed one file earlier.

Checking in suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties,v  <--  composeMsgs.properties
new revision: 1.90; previous revision: 1.89
done
Comment 15 Magnus Melin 2008-03-26 12:48:35 PDT
Created attachment 311842 [details] [diff] [review]
proposed fix (wording change for replyInDefaultCharset)

Change the wording for the mailnews.reply_in_default_charset. 
The behaviour of *the pref* didn't really change, but now the charset is set to utf-8 as send time if need be.

Screen shots coming
Comment 16 Magnus Melin 2008-03-26 12:50:10 PDT
Created attachment 311844 [details]
screenshot thunderbird
Comment 17 Magnus Melin 2008-03-26 12:52:00 PDT
Created attachment 311845 [details]
screenshot seamonkey
Comment 18 neil@parkwaycc.co.uk 2008-03-27 03:50:04 PDT
Comment on attachment 311842 [details] [diff] [review]
proposed fix (wording change for replyInDefaultCharset)

>-<!ENTITY replyInDefaultCharset.label     "Always use this default character encoding in replies. (When unchecked, only new messages use this default.)">
>-<!ENTITY replyInDefaultCharset.accesskey "w">
>+<!ENTITY replyInDefaultCharset2.label    "When possible, use this default character encoding in replies. (When unchecked, only new messages use this default.)">
>+<!ENTITY replyInDefaultCharset2.accesskey "w">
Please change the access key to upper case to match the new text.
Comment 19 Magnus Melin 2008-03-27 11:51:11 PDT
Checking in mailnews/base/prefs/resources/content/pref-character_encoding.xul;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/pref-character_encoding.xul,v  <--  pref-character_encoding.xul
new revision: 1.3; previous revision: 1.2
done
Checking in suite/locales/en-US/chrome/mailnews/pref/pref-character_encoding.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/pref/pref-character_encoding.dtd,v  <--  pref-character_encoding.dtd
new revision: 1.3; previous revision: 1.2
done

Checking in mail/components/preferences/fonts.xul;
/cvsroot/mozilla/mail/components/preferences/fonts.xul,v  <--  fonts.xul
new revision: 1.15; previous revision: 1.14
done
Checking in mail/locales/en-US/chrome/messenger/preferences/fonts.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/preferences/fonts.dtd,v  <--  fonts.dtd
new revision: 1.10; previous revision: 1.9
done

->FIXED
Comment 20 Karsten Düsterloh 2008-04-07 07:07:45 PDT
*** Bug 427525 has been marked as a duplicate of this bug. ***
Comment 21 Rolf Leggewie 2008-08-01 07:56:39 PDT
I generally agree with the approach taken here.  But I want to discuss the following problem and see if we can accomodate it.

I am a German national that frequently writes mails to Japanese users.  For some very strange reasons, the adoption of UTF-8 in Japan is still lacking to say the least.  This patch will lead to a lot of mails that are unreadable on the other end.  Not because the mail is incorrect, but because the recipient's software does not understand UTF-8.  A lot of times this will be out of the recipient's control.  Many Japanese receive mails on their mobile phone.

Can we maybe have a configurable list of encodings (try iso-8859-15 first, then iso-2022-jp, then default to utf-8)?  any possible other ways to solve this?

Comment 22 Karsten Düsterloh 2008-08-01 12:51:15 PDT
(In reply to comment #21)
> This patch will lead to a lot of mails that are unreadable on
> the other end.  Not because the mail is incorrect, but because the recipient's
> software does not understand UTF-8.

This patch only changes the last resort.

> Can we maybe have a configurable list of encodings (try iso-8859-15 first,
> then iso-2022-jp, then default to utf-8)?  any possible other ways to solve
> this?

You already can, that's what the intl.fallbackCharsetList.* pref "family" is for. You can configure global encoding defaults in Mail&News -> Character Encoding, but we don't have a pref panel for configuring custom fallbacks.
In either case, please file new bugs for any problems/regressions with this one.
Comment 23 Rolf Leggewie 2008-08-01 19:08:54 PDT
*** Bug 172061 has been marked as a duplicate of this bug. ***
Comment 24 Rolf Leggewie 2008-08-01 19:28:09 PDT
(In reply to comment #22)
> that's what the intl.fallbackCharsetList.* pref "family" is for.

I am using Thunderbird.  And the above setting does not seem to change
anything.  I opened bug 448842 about the regression issues wrt Japanese users
that this fix introduces.
Comment 25 Gary Kwong [:gkw] [:nth10sd] 2009-08-07 23:58:28 PDT
*** Bug 224391 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.