Last Comment Bug 32714 - [UI] Create a UI for Folder Charset
: [UI] Create a UI for Folder Charset
Status: VERIFIED FIXED
[nsbeta2-]
: intl
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All Other
: P1 normal (vote)
: ---
Assigned To: nhottanscp
: marina
Mentors:
: 55122 61986 (view as bug list)
Depends on:
Blocks: 35851 44060
  Show dependency treegraph
 
Reported: 2000-03-21 13:57 PST by Katsuhiko Momoi
Modified: 2004-11-22 17:25 PST (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
folder property dialog with only charset items (11.04 KB, image/jpeg)
2000-05-22 17:27 PDT, nhottanscp
no flags Details
screen shot of UI (12.57 KB, image/jpeg)
2001-01-09 13:39 PST, nhottanscp
no flags Details
patch for folder charset UI (diff and three new files) (16.46 KB, patch)
2001-01-09 13:40 PST, nhottanscp
no flags Details | Diff | Splinter Review
patch to include timeless suggestion (no exception change) (17.41 KB, patch)
2001-01-09 16:53 PST, nhottanscp
no flags Details | Diff | Splinter Review
patch, removed contributor name and commented out debug dump (17.18 KB, patch)
2001-01-10 10:27 PST, nhottanscp
no flags Details | Diff | Splinter Review
patch, removed try/catch (16.69 KB, patch)
2001-01-10 11:26 PST, nhottanscp
no flags Details | Diff | Splinter Review
patch, updated with suggestions by brendan and seth (16.61 KB, patch)
2001-01-10 13:21 PST, nhottanscp
no flags Details | Diff | Splinter Review

Description Katsuhiko Momoi 2000-03-21 13:57:58 PST
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 2000-03-21 13:58:44 PST
Assigning myself as QA contact.
Comment 2 nhottanscp 2000-03-24 16:40:55 PST
adding depends - 10877 [FEATURE] Folder property dialogs
Comment 3 Kevin Puetz 2000-03-30 05:53:12 PST
*** Bug 33854 has been marked as a duplicate of this bug. ***
Comment 4 Kevin Puetz 2000-03-30 05:56:28 PST
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 2000-04-04 11:41:27 PDT
Beta2 feature, so marked M16, but depends upon a bug marked M17...
   bug 10877 "[FEATURE] UI: Folder property dialogs"
Comment 6 bobj 2000-05-02 14:15:10 PDT
Added jglick to cc list to get UE input.
Comment 7 jglick 2000-05-03 14:28:59 PDT
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 2000-05-03 16:56:59 PDT
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 2000-05-04 15:01:19 PDT
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 2000-05-04 16:27:23 PDT
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 2000-05-04 16:52:03 PDT
With the above modified UI proposal, the check mark will be
OFF by default.
Comment 12 simone 2000-05-08 11:42:41 PDT
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! simone@netscape.com
Does a functional spec of this feature exist anywhere
Comment 13 jglick 2000-05-08 14:10:38 PDT
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 2000-05-08 14:37:21 PDT
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 2000-05-08 15:26:05 PDT
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 2000-05-09 22:44:36 PDT
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 2000-05-16 11:43:17 PDT
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 2000-05-17 09:29:09 PDT
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 2000-05-22 17:26:37 PDT
>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 2000-05-22 17:27:34 PDT
Created attachment 9015 [details]
folder property dialog with only charset items
Comment 21 bobj 2000-05-22 17:47:02 PDT
jglick,  See nhotta's attached image.  Does this look OK for you for Beta2?
Comment 22 jglick 2000-05-23 09:59:30 PDT
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 2000-05-23 14:35:32 PDT
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 2000-05-23 15:38:42 PDT
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 2000-05-23 17:51:41 PDT
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 2000-05-23 17:52:33 PDT
Need a preposition:

[] Apply this default to all messages ignoring Character Coding info 
   in the messages.
Comment 27 Katsuhiko Momoi 2000-05-25 02:01:39 PDT
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 2000-05-25 02:04:57 PDT
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 2000-05-30 17:51:33 PDT
Putting on [nsbeta2-] radar. Removing 5/16. Out for Netscape 6.
Comment 30 Mike 2000-06-21 13:38:21 PDT
M16 has been out for a while now, these bugs target milestones need to be 
updated.
Comment 31 bobj 2000-06-27 16:15:44 PDT
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 2000-06-27 16:17:16 PDT
Corrected platform/OS fields.  This bug is XP.
Comment 33 Frank Tang 2000-06-28 01:07:38 PDT
reassign to nhotta. Put into the 6.1 radar please.
Comment 34 Frank Tang 2000-07-28 13:01:16 PDT
take out nsbeta2
Comment 35 Daniel Veditz [:dveditz] 2000-08-09 13:20:42 PDT
Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the 
queries don't get screwed up
Comment 36 robinf 2000-08-11 14:39:53 PDT
Adding myself to cc: list.
Comment 37 nhottanscp 2000-10-03 17:18:50 PDT
*** Bug 55122 has been marked as a duplicate of this bug. ***
Comment 38 Katsuhiko Momoi 2000-12-05 17:07:04 PST
*** Bug 61986 has been marked as a duplicate of this bug. ***
Comment 39 nhottanscp 2000-12-21 11:03:59 PST
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 2001-01-07 23:41:08 PST
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 2001-01-08 22:44:36 PST
QA contact to marina.
Comment 42 nhottanscp 2001-01-09 13:39:04 PST
Created attachment 22170 [details]
screen shot of UI
Comment 43 nhottanscp 2001-01-09 13:40:21 PST
Created attachment 22171 [details] [diff] [review]
patch for folder charset UI (diff and three new files)
Comment 44 nhottanscp 2001-01-09 13:45:14 PST
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 2001-01-09 15:09:07 PST
+  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 <hidday@geocities.com>
if he's a contributor, have you asked him for permission to use NPL? ie 
shouldn't it be MPL?
Comment 46 nhottanscp 2001-01-09 15:28:42 PST
>Fabian Guisset <hidday@geocities.com> 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 hidday@geocities.com 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 2001-01-09 15:45:18 PST
blake/brendan, care to explain exceptions? I don't know of a reference :-(
Comment 48 nhottanscp 2001-01-09 16:53:54 PST
Created attachment 22205 [details] [diff] [review]
patch to include timeless suggestion (no exception change)
Comment 49 (not reading, please use seth@sspitzer.org instead) 2001-01-09 17:11:13 PST
suggestions:

1) you should open up that dialog as "centered".
2) comment out your debugging dump() statements
Comment 50 nhottanscp 2001-01-09 17:22:14 PST
The dialog is centered by moveToAlertPosition().
Why need to comment debug, does that affect performance of release build?
Comment 51 Fabian Guisset 2001-01-09 22:51:40 PST
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 2001-01-10 10:27:46 PST
Created attachment 22255 [details] [diff] [review]
patch, removed contributor name and commented out debug dump
Comment 53 Brendan Eich [:brendan] 2001-01-10 11:10:02 PST
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 2001-01-10 11:26:18 PST
Created attachment 22261 [details] [diff] [review]
patch, removed try/catch
Comment 55 (not reading, please use seth@sspitzer.org instead) 2001-01-10 12:00:28 PST
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] 2001-01-10 12:25:53 PST
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 2001-01-10 13:21:47 PST
Created attachment 22272 [details] [diff] [review]
patch, updated with suggestions by brendan and seth
Comment 58 Brendan Eich [:brendan] 2001-01-10 13:26:07 PST
+              {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=brendan@mozilla.org in any case.
Should mscott or bienvenu sr=?

/be
Comment 59 David :Bienvenu 2001-01-10 13:33:39 PST
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 seth@sspitzer.org instead) 2001-01-10 13:35:33 PST
MANIFESTs are obsolete in this case, since it is all in messenger.jar
Comment 61 nhottanscp 2001-01-10 13:37:52 PST
How about makefile.win?
Comment 62 timeless 2001-01-10 13:44:05 PST
bryner wants to cvs remove makefile.win from chrome only places.
Comment 63 nhottanscp 2001-01-10 13:49:24 PST
Okay, I won't check in changes for MANIFEST and makefile.win.
Comment 64 (not reading, please use seth@sspitzer.org instead) 2001-01-10 13:51:32 PST
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 Paul Thomas 2001-01-10 15:54:01 PST
Why is this in its own dialog, and not part of the Folder Properties window?
Comment 66 timeless 2001-01-10 16:21:19 PST
because when we last discussed it, that window was unimplemented. please reread 
the original nightmarish discussion.
Comment 67 bobj 2001-01-10 16:24:26 PST
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 2001-01-10 16:27:00 PST
I checked in the dialog. Note that override option does not function until I fix 
bug 39756.
Comment 69 (not reading, please use seth@sspitzer.org instead) 2001-01-10 20:29:22 PST
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 seth@sspitzer.org instead) 2001-01-10 20:35:26 PST
new bugs:

http://bugzilla.mozilla.org/show_bug.cgi?id=65018
http://bugzilla.mozilla.org/show_bug.cgi?id=65019
Comment 71 nhottanscp 2001-01-11 10:05:45 PST
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 seth@sspitzer.org instead) 2001-01-11 10:38:15 PST
taking this discussion to #65018

see my reply to your comments in that bug.
Comment 73 marina 2001-01-23 16:32:19 PST
UI for folder charset is in and functionning, verifying

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