32714 - [UI] Create a UI for Folder Charset

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P1)

Description

[Katsuhiko Momoi]

Bug 31058 deals with backend work to assign a folder charset
attribute. We need a UI design to make use of this feature.

I would like to begin with a simple design like the following
and revise as needed based on feedback.

In the Folder Properties dialog, there should be the following items:

a. A list of available charsets for viewing msgs in a combobox.
b. Two radio buttons with the following captions:

   1. Charset default for this folder. (Applies only when charset info
      is not available in the message under view.)
   2. Charset default for this folder. (Override message charset and 
      apply this default to all messages.)

Note: There will be another charset default setting for all the folders.
      This pref is to be placed under Mail\News\Intl setting area.
      At the initial stage, all folders default will be set to this
      one. Then via this proposed UI, the user can set per folder
      charset default.

Comment 1

[Katsuhiko Momoi]

Assigning myself as QA contact.

Comment 2

[nhottanscp]

adding depends - 10877 [FEATURE] Folder property dialogs

Comment 3

[Kevin Puetz]

*** Bug 33854 has been marked as a duplicate of this bug. ***

Comment 4

[Kevin Puetz]

sorry, that's not a dup, it's a typo (it was supposed to be a dup of 32741). I
fixed it, please disregard that last bug 33854 (unless someone can delete the
comment)

Comment 5

[bobj]

Beta2 feature, so marked M16, but depends upon a bug marked M17...
   bug 10877 "[FEATURE] UI: Folder property dialogs"

Comment 6

[bobj]

Added jglick to cc list to get UE input.

Comment 7

[jglick]

I'm not sure if I understand the exact purpose of the two radio buttons. Is what 
I have below correct?

cc'ing Simone for assistance with wording.
-----------------

Character Set: Dropdown Menu of available choices

Apply this character set to messages in this folder:
 (*)As needed. Applies only when charset info is not available in the message 
under view
 ( )Always. Override message charset and apply this default to all messages.

Comment 8

[Katsuhiko Momoi]

Yes. I think you have it right in these 2 options. 
These 2 should take care of our concerns but do we
have specs for the rest of folder properties? We would
like to see how these will fit into the rest of the 
dialog items and see if we need to make any adjustment.

Comment 9

[Katsuhiko Momoi]

Naoki suggest that we have a checkbox instead. 
I think that will work better than my original proposal
since each folder willhave to have a charset property any way.
The real question is whether or not that charset should override
charset parameter in the mail. So modifying jglick's UI somewhat,

somthing like the following would be simpler.

============================
Character Set: Dropdown Menu of available choices

 [] Override message charset and apply this default charset to all messages.

============================

Comment 10

[jglick]

There is a draft spec here:
http://gooey/client/5.0/specs/mail/messenger/FolderProperties.html

But I'm waiting for feedback from the mail team to see what 4.x features will 
need to be in and which are out.

Comment 11

[Katsuhiko Momoi]

With the above modified UI proposal, the check mark will be
OFF by default.

Comment 12

[simone]

As the doc person for mail, I have some questions about this feature, such as 
does it appear anywhere else? What would be the typical usage of this 
feature? Does it apply to mail messages only or newsgroup messages? What happens 
if the user moves a message to a folder with a different charset than the 
original? 
If there is a functional spec for this feature, please let me know, as I'm the 
one who'll have to describe how it works. There is a reference made to a 
preference that will appear under the Mail & Newsgroup category, but I've 
not heard of any such thing. 
Thanks! [e-mail]
Does a functional spec of this feature exist anywhere

Comment 13

[jglick]

Is this closer to are trying to get at?

Checkbox: Override the message charset for all messages in this folder
    Character Set: Dropdown Menu of available choices (disabled if checkbox 
    above not checked).

Bug 10877 "[FEATURE] UI: Folder property dialogs":  Looks like this has been 
marked M20, and hence out of Netscape 6.  If 10877 isn't done, then this one 
can't be done either?

Comment 14

[Katsuhiko Momoi]

Hi, jglick, we can use the language you suggest with the following change:

Character Set: Dropdown Menu of available choices  (Note; this list is abled whether or not the checkbox is
                                                                                    checked.)
   Checkbox: Override the message charset for all messages in this folder

The checkbox is secondary to the list and should follow the Drop down list.
I still like the language and layout I suggested on 5/4 better than this one.

Bug 10877 is not directly related to this feature and should not affect 
when this one should get done, This is needed for nsbeta2.

Comment 15

[leger]

Putting on [nsbeta2+][5/16] radar.  This is a feature MUST complete work by 
05/16 or we may pull this feature for PR2.

Comment 16

[Katsuhiko Momoi]

A reply to Simone:

(I hope to write down the rest of the International Messenger
   related specs but for now the following will have to do. Engineering
   specs exist in Naoki Hotta's page under:
    http://www.mozilla.org/projects/intl .)

1. Does folder charset UI appear anywhere other than 
   in the folder properties? -->  Yes, if Bug 32720 gets
   implemented. That Pref UI allows an en mass change to all existing
   folders and future folders.

2. Typical usage: Most messages have MIME charset indicated in
     mail headers. We honor that first. But if a message lacks
     the charset info, and if auto-detection is not used or
     has not succeeded, we use the folder charset as the last fallbck.
     Typically, Japanese users will have this set to Japanese and
     so unlabeled msgs will be regarded as Japanese ones and displayed
     with appropriate fonts for it.

3. Apply to both mail and news folders. We anticipate much more
   heavy use in news where charset info in headers is often missing
   and users are mroe like to set folder charset specially. In some 
   languages like Chinese, different newsgroups have different default 
   charset used.
   There, being able to set the charset "per folder" will come in handy.
   (Note: This per folder features is not currently available in OE5.)

4. What happens if the user moves a message to a folder with a 
   different charset than the original? --> 
    In that case, if that message has MIME charset info in the headers,
    nothing changes. If it does not contain MIME charset info and
    if auto-detction is not used or fails, then the folder charset 
    will be used sa teh fallback and thus the display result would 
    be different between the 2 folders.
5. In this dialog, we also allow the user to override even the MIME
   charset info contained in the messages. When this option is checked
   off, Messenger will regard all messages to be in that charset.
   We've heard from several net users that they would like this feature,
   "Consider all messages in this folder to be in Japanese."

6. The Pref UI item will allow you to set a new default upon creating
   a new profile on pre-set folders like Inbox, and then the default for
   all new folders to be. If you used per-folder charset options
   before on existing folders, the Pref UI will reset all of them
   to a different one by one action.

Comment 17

[bobj]

jglick,

It looks like bug 10877 "[FEATURE] UI: Folder property dialogs" is out
(based upon the comment in that bug from 2000-05-06 10:46).
But we still need the ability for the user the modify the folder charset.

Currently, there is the File|Rename Folder... menu item which pops up a
dialog.  Would it make sense to modify this to a File|Folder Properties...,
but with reduced functionality: renaming and modifying the folder charset?
It would be easy for us to add a drop down for the folder charset to this
existing dialog and change the name of the menu entry.  (This is similar
to your spec
  http://gooey/client/5.0/specs/mail/messenger/FolderProperties.html
except no info on location, type, unread/total messages and space usage.)

Comment 18

[jglick]

I wouldn't recommend placing this pref on the File/Rename folder dialog and 
changing the name of this dialog to "Folder Properties".  

Why not just have the Edit/Folder Properties dialog, but for B2 it would only 
contain the character set stuff?  This is much less complex that trying to 
change another dialog that already has a set purpose.

Or maybe  Bug 10877 "[FEATURE] UI: Folder property dialogs" needs to be nsbeta2+ 
because this feature is dependent on it?

Comment 19

[nhottanscp]

>Why not just have the Edit/Folder Properties dialog, but for B2 it would only 
>contain the character set stuff? 
Available on my local tree (need additional 0.5 day to clean up), I will attach 
a screen shot.

Comment 20

[nhottanscp]

Attachment: folder property dialog with only charset items

Comment 21

[bobj]

jglick,  See nhotta's attached image.  Does this look OK for you for Beta2?

Comment 22

[jglick]

This is fine for PR2.  One comment, should "charset" in the checkbox sentence 
say "character coding" to match the drop down menu above it?

Comment 23

[Katsuhiko Momoi]

No, the current wording is correct since the "message charset"
refers to the "charset" parameter in the Content header. 
We might make it explicit and say "charset parameter". but "charset"
should do. 

Comment 24

[bobj]

The wording is technically correct, but may be confusing to the user.

Would it would be clearer to have radio buttons instead of the checkbox?
Something like:

  Determine character coding for messages in this folder from
    o the charset header of each message
    o the folder character coding (overriding the charset header)

Comment 25

[Katsuhiko Momoi]

Well, these 2 choices make the connection to the Character Coding
selection obscure. I think there is a reason why this item 
needs to be a checkbox. I would rather perfer something like
the following:

[] Apply this default to all messages ignoring Character Coding info 
   the messages.

Comment 26

[Katsuhiko Momoi]

Need a preposition:

[] Apply this default to all messages ignoring Character Coding info 
   in the messages.

Comment 27

[Katsuhiko Momoi]

Bob, what do you think of a variation on your radio button
proposal on this? Something like:

---------------------------------------------------
Default Character Coding: [List of selections]

    o Apply this default only if the message header lacks
       Character Coding info.
    o Apply this default ignoring Character Coding info 
       in the messages.
---------------------------------------------------

The 1st radio button will be always checked as the default.
This way, the connection to the Selection List is unified and
the default choice is offered untill the user manually changes it.

Comment 28

[Katsuhiko Momoi]

An addition of "all" to clarify the meaning:

---------------------------------------------------
Default Character Coding: [List of selections]

    o Apply this default only if the message header lacks
       Character Coding info.
    o Apply this default to all the messages ignoring Character 
       Coding info in them.
---------------------------------------------------

Comment 29

[leger]

Putting on [nsbeta2-] radar. Removing 5/16. Out for Netscape 6.

Comment 30

[Mike]

M16 has been out for a while now, these bugs target milestones need to be 
updated.

Comment 31

[bobj]

This should get fixed for the first point release after we complete the first
release of Mozilla/Netscape6 (6.01?).

Here is the compromise solution for 6.0:

A checkbox (default off) would be added to the
Edit|Preferences|Viewing Messages|Languages (default is unchecked): 

    [ ] Apply default to all messages (ignore charset specified by MIME header)

This compromise resolves the issue of providing a mechanism (in the
non-default settings) for persistent overriding of messages with
mislabeled charsets, but it does not provide folder level control for the
handling of both mislabeled and unlabeled messages.

See also bug 43101
     Cannot read Chinese newsgroup unless reset charset per message
     message 

Comment 32

[bobj]

Corrected platform/OS fields.  This bug is XP.

Comment 33

[Frank Tang]

reassign to nhotta. Put into the 6.1 radar please.

Comment 34

[Frank Tang]

take out nsbeta2

Comment 35

[Daniel Veditz [:dveditz]]

Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the 
queries don't get screwed up

Comment 36

[robinf]

Adding myself to cc: list.

Comment 37

[nhottanscp]

*** Bug 55122 has been marked as a duplicate of this bug. ***

Comment 38

[Katsuhiko Momoi]

*** Bug 61986 has been marked as a duplicate of this bug. ***

Comment 39

[nhottanscp]

The current plan is to have a dialog similar to "Remane Folder" dialog,
"View" -> "Folder Character Coding...", above "Charset Coding" menu item.
The dialog to contain a menu with list of character codings and a checkbox to
indicate override (i.e. force to apply the character coding to all the messages
in the folder).

Comment 40

[Katsuhiko Momoi]

I would like to suggest that we do something to make
it easy for the user to discover that per folder encoding change
changes the thread pane display. See my comments in 
Bug 46362.

Comment 41

[Katsuhiko Momoi]

QA contact to marina.

Comment 42

[nhottanscp]

Attachment: screen shot of UI

Comment 43

[nhottanscp]

Attachment: patch for folder charset UI (diff and three new files)

Comment 44

[nhottanscp]

The patch implements folder charset UI. The dialog contains a list of charset
and a override checkbox. The folder charset and override flag are stored to the
database.
Seth, could you review the patch?

Comment 45

[timeless]

+  catch(e)
+  {
+    dump ("Exception : MsgSetFolderCharset \n");
+  }
wouldn't it be better to use standard exception handling than to catch the 
exception?

+  if (folderTree)
+  {
+    if (uri && (uri != "") && charset && (charset != ""))
+    {
i'd prefer:
+  if (folderTree) {
+    if (uri && charset) {
the { is a prefered style, the omission of !="" is tied to the way javascript 
strings/objects are handled. null, "" and others are treated as false.

The above comments apply throughout the patch.

The prefered brace style is to only put them on a newline for functions.

===================================================================
RCS file: /cvsroot/mozilla/mailnews/base/resources/locale/en-US/messenger.dtd,v
something messed up? you have the dtd and then i guess the new files.

cvs add filename
cvs diff -u -N filename

                <checkbox id="folderCharsetOverride" 
value="&folderCharsetOverride.label;" />
is indented too far

Fabian Guisset <[e-mail]>
if he's a contributor, have you asked him for permission to use NPL? ie 
shouldn't it be MPL?

Comment 46

[nhottanscp]

>Fabian Guisset <[e-mail]> if he's a contributor, have you asked him
>for permission to use NPL? ie
>shouldn't it be MPL?
The header part was just inherited as I used renameFolderDialog.dtd as an
template, cc to [e-mail] if it's okay to put the name there.

>wouldn't it be better to use standard exception handling than to catch the
>exception?
Sorry, I don't know about the standard exception handling, could you explain or
point me a reference?

Comment 47

[timeless]

blake/brendan, care to explain exceptions? I don't know of a reference :-(

Comment 48

[nhottanscp]

Attachment: patch to include timeless suggestion (no exception change)

Comment 49

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

suggestions:

1) you should open up that dialog as "centered".
2) comment out your debugging dump() statements

Comment 50

[nhottanscp]

The dialog is centered by moveToAlertPosition().
Why need to comment debug, does that affect performance of release build?

Comment 51

[Fabian Guisset]

I am very flattered that you use my name as a contributor to all your new files, 
but since I did nothing at all for those files (only newFolderDialog.js/xul and 
renameFolderDialog.js/xul), my name should be removed from the headers, and 
since they are your files, you are free to use whatever license you want.
Thanks for thinking about that,
Fabian.

Comment 52

[nhottanscp]

Attachment: patch, removed contributor name and commented out debug dump

Comment 53

[Brendan Eich [:brendan]]

nhotta: timeless makes a good point -- why catch exceptions that should
propagate and stop the script or event handler in its tracks?  Do you really
expect GetCharsetTitle, or setting folder charset, to fail (in the XPCOM sense
of returning an NS_FAILED(rv) return value)?  Why not eliminate those try/catch
statements and let the chips fall where they may?

/be

Comment 54

[nhottanscp]

Attachment: patch, removed try/catch

Comment 55

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

r=sspitzer

just curious, if you don't call moveToAlertPosition(), and you add "centered" to
your window.openDialog() call, will that do the right thing?  if so, is that
preferred way to center a dialog?

Comment 56

[Brendan Eich [:brendan]]

Instead of

  dialog = {};

  dialog.OKButton = document.getElementById("id");

etc., use the object initializer to define the properties (it's slightly faster,
and it's better style):

  dialog = {
    OKButton: document.getElementById("id"),
    okCallback: arguments.okCallback,
    . . .
  };

Nit: declare var charsetTitle = ...; rather than var charsetTitle; ...;
charsetTitle = ...;

Ok, I see an object initializer used well, in the call to openDialog -- but how
about formatting that one so each property initializer is on its own line?  The
second through last properties all pile up on the second, long-ish line, and
don't underhang the first property init (the second line isn't indented by one
more space, in other words).

Sorry for picking nits.  A mailnews XUL guru should review the rest.

/be

Comment 57

[nhottanscp]

Attachment: patch, updated with suggestions by brendan and seth

Comment 58

[Brendan Eich [:brendan]]

+              {preselectedURI: preselectedURI,

+              okCallback: SetFolderCharset, folderCharset: msgFolder.charset,
folderCharsetOverride: msgFolder.charsetOverride});


This was the formatting I whined about last time.  Wouldn't

+              {preselectedURI: preselectedURI,

+               okCallback: SetFolderCharset,
+               folderCharset: msgFolder.charset,
+               folderCharsetOverride: msgFolder.charsetOverride});


be a little easier on the eyes?  Thanks!  r=[e-mail] in any case.
Should mscott or bienvenu sr=?

/be

Comment 59

[David :Bienvenu]

do you need to add folderCharsetDialog.dtd to the MANIFEST, or are MANIFEST's
obsolete? If so, sr=bienvenu

Comment 60

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

MANIFESTs are obsolete in this case, since it is all in messenger.jar

Comment 61

[nhottanscp]

How about makefile.win?

Comment 62

[timeless]

bryner wants to cvs remove makefile.win from chrome only places.

Comment 63

[nhottanscp]

Okay, I won't check in changes for MANIFEST and makefile.win.

Comment 64

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

with jar.mn, both the Mac MANIFEST files, the win32 makefile.win files, and the
UNIX Makefile.in files for chrome directories do not need to list out all the files.

see #55778 for details.

Comment 65

[Matthew T (active 1999-2002)]

Why is this in its own dialog, and not part of the Folder Properties window?

Comment 66

[timeless]

because when we last discussed it, that window was unimplemented. please reread 
the original nightmarish discussion.

Comment 67

[bobj]

Seconding timeless' comments.

That was the original request, but the Folder Properties UI was killed.
     bug 10877 "[FEATURE] UI: Folder property dialogs"

If that feature is revived then as mpt suggests, the Folder Charset belongs
there.

Comment 68

[nhottanscp]

I checked in the dialog. Note that override option does not function until I fix 
bug 39756.

Comment 69

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

good news is that there is a folder properties dialog now.

There is no Edit | Properties yet, but you can get to it by right clicking on
the folder.

I'll open a new bugs on adding "Edit | Properties", and on nhotta to add this
functionality to that dialog.

sorry nhotta, for not thinking of this earlier.

Comment 70

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

new bugs:

http://bugzilla.mozilla.org/show_bug.cgi?id=65018
http://bugzilla.mozilla.org/show_bug.cgi?id=65019

Comment 71

[nhottanscp]

I think having folder charset UI close to charset menu make it easier to be
discovered by the users who care about charset.
We can have folder charset items in folder property but we can also keep the
current UI.

Comment 72

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

taking this discussion to #65018

see my reply to your comments in that bug.

Comment 73

[marina]

UI for folder charset is in and functionning, verifying