39756 - [FEATURE] folder level charset override support (back end)

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect, P1)

Description

[nhottanscp]

This is separated from 5938 (charset override) and related to 32714 (folder 
charset UI).
We need a functionality of folder level override charset which forces all 
messages in a folder to use one override charset.
In the back end code, need a flag in a folder attribute (same place as the 
charset is stored), default as false. When the flag is on, a folder charset to 
be always set to override charset field. Only the manual override should 
override that setting.

Comment 1

[nhottanscp]

Nominating for nsbeta2.

Comment 2

[Scott MacGregor]

Naoki, I'm *very* confused. I just implemeted a feature for you to pass the
folder charset down for messages. I don't understand how this differs from that
feature I just implemented for you guys.

In addition, it's a bit late to be writing new feature bugs after the beta2
feature freeze. =(

Comment 3

[nhottanscp]

Sorry about this has not been explained clearly.
The current implementation of the folder charset matches what we requested.
This bug is about an option of using a folder charset as a override charset. 
Depends on the language, the charset label is mostly wrong. That case, the user 
want to override entire folder by one charset instead of manually overriding 
each messages. This has been proposed in UI (32714) but not been specified 
clearly in the back end bug.

Comment 4

[Scott MacGregor]

adding feature to the summary

Comment 5

[leger]

Putting on [nsbeta2-] radar.   Pulling from beta2 per PDT mtg today.

Comment 6

[Scott MacGregor]

not in beta2 according to pdt...that means m18 or later...

Comment 7

[Scott MacGregor]

is the UI a beta3 approved thing or was that futured 'cause it didn't make the
feature exception date? If it was futured, then I'd like to mark this bug future
as well. 

Comment 8

[nhottanscp]

The folder charset UI bug 32714 is marked as future.

Comment 9

[nhottanscp]

Reassign to nhotta, this is needed for bug 32714.
I probably need a help of mscott to implement this.

Comment 10

[nhottanscp]

Attachment: patch 1: change for mailnews/db, added override field to db

Comment 11

[nhottanscp]

Attachment: patch 2: change for mailnews/base, added override field to db

Comment 12

[nhottanscp]

Attached two patches are for adding override field to db. I got a sr=bienvenu, I
need one more review.
I also need a separate change to add override filed to nsIMsgMessageUrl.

Comment 13

[(not reading, please use [e-mail] instead)]

r=sspitzer

I notice you commit the db after setting the char set.  if that is ok with
bienvenu, that is ok with me.

Comment 14

[David :Bienvenu]

sr=bienvenu - it's ok to commit, as long as you only call SetCharsetoverride as
the result of a user interaction, e.g., a menu command.

Comment 15

[Katsuhiko Momoi]

QA contact to ji.

Comment 16

[nhottanscp]

Attachment: patch3: added folderCharsetOverride to nsIMsgI18NUrl and related changes

Comment 17

[nhottanscp]

I added folderCharsetOverride to nsIMsgI18NUrl and used it in libmime,
implemented GetFolderCharsetOverride() for imap, local and news (almost the same
implementation as GetFolderCharset().
Seth, please review the patch.

Comment 18

[(not reading, please use [e-mail] instead)]

no go on that patch.

instead of duplicating the code to get the nsIMsgFolder from the uri, you should
move that bit code out into a separate method (for all three url
implementations) and have GetFolderCharsetOverride() and GetFolderCharset() both
call it.

Comment 19

[nhottanscp]

Attachment: new patch, rewritten with the suggestion by seth

Comment 20

[(not reading, please use [e-mail] instead)]

you should also check the return value of your calls to GetMsgFolder.

so

+  GetMsgFolder(getter_AddRefs(folder));
+  NS_ENSURE_TRUE(folder, NS_ERROR_FAILURE);

should be

+  rv = GetMsgFolder(getter_AddRefs(folder));
+  NS_ENSURE_SUCCESS(rv,rv);
+  NS_ENSURE_TRUE(folder, NS_ERROR_FAILURE);

also, instead of

+  nsCOMPtr<nsIMsgFolder> folder = do_QueryInterface(resource, &rv);
+  NS_ENSURE_SUCCESS(rv,rv);
+  *msgFolder = folder;

how about:

+ rv = folder->QueryInterface(NS_GET_IID(nsIMsgFolder), (void**) msgFolder);
+ NS_ENSURE_SUCCESS(rv,rv);

the QI will do the addref, and you don't need to create and destroy the comptr.

comments?

Comment 21

[nhottanscp]

Attachment: updated patch with the suggestion by seth

Comment 22

[(not reading, please use [e-mail] instead)]

r=sspitzer

Comment 23

[David :Bienvenu]

sr=bienvenu

Comment 24

[nhottanscp]

checked in

Comment 25

[msanz]

Naoki, can you verify this bug? If so, please change the QA contact.

Comment 26

[nhottanscp]

Attachment: test data, contains ISO-2022-JP message incorrectly labeled as ISO-8859-1

Comment 27

[nhottanscp]

Verified.
Used win32 build ID 2001012205.
Used the attached data which is not readable, I set a folder charset to
ISO-2022-JP and set an override flag then I was able to see the body as correct
Japanese.