694514 - Expose mailnews.reply_header_type preference

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, enhancement)

Description

[Ian Neal]

There exists the ability to change the reply header type using the mailnews.reply_header_type preference:
// 0 - No Reply-Text
// 1 - "[Author] wrote:"
// 2 - "On [date] [author] wrote:"
// 3 - User-defined reply header.

Bug 107884 deals with configuring when set it is set to 3, but we should expose this preference to users with some sort of warning/note when users set it to 3 for the moment.

Comment 1

[rsx11m]

Actually, the "3" switches the order to show mailnews.reply_header_authorwrote followed by mailnews.reply_header_ondate and is the default in some locales for Thunderbird where you need a "<name> wrote on <date>" order.

There is currently a placeholder in nsMsgCompose.cpp for "4" being the value for free user-defined headers, which may or may not be implemented in bug 107884.

Comment 2

[Raymond]

I suggest:
// 4 - same as when we do a "Forward".

Comment 3

[rsx11m]

As long as "4" doesn't perform any function it should be just omitted from any UI implemented here. It should be easy to add later, e.g., as another menu item.

Comment 4

[Edmund Wong (:ewong)]

Attachment: Expose mailnews.reply_header_type preference. (v1)

Added to Mail & Newsgroups -> Composition.

Comment 5

[Edmund Wong (:ewong)]

Attachment: Expose mailnews.reply_header_type preference. (v2)

Comment 6

[Ian Neal]

Comment on attachment 601565 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v2)

Have a look at:
http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#2299
which explains what each value does better than I did.

Comment 7

[Edmund Wong (:ewong)]

Attachment: Expose mailnews.reply_header_type preference. (v3)

Comment 8

[Raymond]

Is it possible to add:
+              <menuitem value="5"
+                        label="&sameAsForwardOption.label;"/>

Comment 9

[Ian Neal]

(In reply to Raymond from comment #8)
> Is it possible to add:
> +              <menuitem value="5"
> +                        label="&sameAsForwardOption.label;"/>

5 is not a valid option in the backend as far as I am aware.

Comment 10

[Raymond]

In fact - we were expecting to have &sameAsForwardOption.label;"/>
instead of one of the others ...

Comment 11

[rsx11m]

Raymond - if it's not supported in the backend (and indeed it is *not*) it doesn't make sense to add a label to an option which isn't implemented. Thus, your efforts are moot here.

BTW: That's bug 218258 where you'll need to advocate for it, the only reason why it showed up in bug 107884 was that it would have been a good/comprehensive template to use as a default.

Comment 12

[Raymond]

Is it so difficult to implement it, when we know that the coding is already done when forwarding. It's a pity situation when we see that nobody take this https://bugzilla.mozilla.org/show_bug.cgi?id=218258 evenwhile with 32 votes instead of 2 here - and also because it's 8 years ...old.

Comment 13

[rsx11m]

> http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#2320

Option #4 currently just defaults to whatever #1 does, thus I'm wondering if it should be offered at this time or just hidden pending any action in bug 107884. Your patch shows it right now but then adds another label explaining that it's not implemented yet, which would have to be removed once it actually has been implemented. So, either way it means follow-up work whenever more options become available.

Comment 14

[rsx11m]

Previous comment was directed to Edmund, comment #12 is off-topic.

Comment 15

[Ian Neal]

Comment on attachment 601899 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v3)

>+++ b/suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd

>+<!ENTITY userDefinedOption.label              "User-defined reply header">
>+<!ENTITY userDefinedNoticeValue.label         "User defined reply header is not yet implemented">
I'd not bother with the user defined option here.

>+++ b/suite/mailnews/compose/prefs/pref-composing_messages.xul
>+            <menupopup>
>+              <menuitem value="0"
>+                        label="&noReplyOption.label;"/>
>+              <menuitem value="1"
>+                        label="&authorWroteOption.label;"/>
>+              <menuitem value="2"
>+                        label="&onDateWroteOption.label;"/>
>+              <menuitem value="3"
>+                        label="&authorOnDateWroteOption.label;"/>
>+              <menuitem value="4"
>+                        label="&userDefinedOption.label;"/>
>+              <menuseparator/>
No need for a menuseparator here (or user defined).

r=me with those fixed.

Comment 16

[Edmund Wong (:ewong)]

Attachment: Expose mailnews.reply_header_type preference. (v4)

Comment 17

[Edmund Wong (:ewong)]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/9f6c12aa2efb

Comment 18

[Thomas D. (:thomas8)]

Edmund, can you attach screenshot of implemented SeaMonkey solution?

Is this something Thunderbird could/should do, too?

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 602710 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v4)

Review of attachment 602710 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
@@ +71,5 @@
> +<!ENTITY selectHeaderType.label               "Select reply header type :">
> +<!ENTITY selectHeaderType.accesskey           "e">
> +<!ENTITY noReplyOption.label                  "No Reply Header">
> +<!ENTITY authorWroteOption.label              "[Author] wrote:">
> +<!ENTITY onDateWroteOption.label              "On [date] [author] wrote:">

Care to s/onDateWroteOption/onDateAuthorWroteOption/g?

Comment 20

[rsx11m]

(In reply to Serge Gautherie (:sgautherie) from comment #19)
> Care to s/onDateWroteOption/onDateAuthorWroteOption/g?

Agreed, that's longer but would be more consistent.

Comment 21

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 602710 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v4)

Review of attachment 602710 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
@@ +67,5 @@
>  <!ENTITY fontColor.accesskey                  "T">
>  <!ENTITY bgColor.label                        "Background:">
>  <!ENTITY bgColor.accesskey                    "B">
> +
> +<!ENTITY selectHeaderType.label               "Select reply header type :">

Ian, are you really OK with the space before the colon or did you miss it? I think it looks terrible.

Comment 22

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 602710 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v4)

Review of attachment 602710 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
@@ +72,5 @@
> +<!ENTITY selectHeaderType.accesskey           "e">
> +<!ENTITY noReplyOption.label                  "No Reply Header">
> +<!ENTITY authorWroteOption.label              "[Author] wrote:">
> +<!ENTITY onDateWroteOption.label              "On [date] [author] wrote:">
> +<!ENTITY authorOnDateWroteOption.label        "[Author] on [date] wrote:">

onDateWroteOption.label should really be "On [date], [author] wrote:", and authorOnDateWroteOption.label should really be "[Author] wrote, On [date]:".

Just go through the options yourself and check what the effect is. I did, hence the corrections.

Comment 23

[rsx11m]

If you want to do it really sophisticated you'd get the current values of mailnews.reply_header_{authorwrote,colon,ondate,separator} and build those labels on the fly, but that may be overdoing it.

Comment 24

[Ian Neal]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #21)
> Comment on attachment 602710 [details] [diff] [review]
> Expose mailnews.reply_header_type preference. (v4)
> 
> Review of attachment 602710 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
> @@ +67,5 @@
> >  <!ENTITY fontColor.accesskey                  "T">
> >  <!ENTITY bgColor.label                        "Background:">
> >  <!ENTITY bgColor.accesskey                    "B">
> > +
> > +<!ENTITY selectHeaderType.label               "Select reply header type :">
> 
> Ian, are you really OK with the space before the colon or did you miss it? I
> think it looks terrible.
Yes, I did miss the space, sorry.

Comment 25

[Ian Neal]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #22)
> Comment on attachment 602710 [details] [diff] [review]
> Expose mailnews.reply_header_type preference. (v4)
> 
> Review of attachment 602710 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
> @@ +72,5 @@
> > +<!ENTITY selectHeaderType.accesskey           "e">
> > +<!ENTITY noReplyOption.label                  "No Reply Header">
> > +<!ENTITY authorWroteOption.label              "[Author] wrote:">
> > +<!ENTITY onDateWroteOption.label              "On [date] [author] wrote:">
> > +<!ENTITY authorOnDateWroteOption.label        "[Author] on [date] wrote:">
> 
> onDateWroteOption.label should really be "On [date], [author] wrote:", and
> authorOnDateWroteOption.label should really be "[Author] wrote, On [date]:".
> 
I did notice this but somehow I lost the comment. It did include talking about spinning off a bug for tweaking the format of these options and changing what SM's default should be (I think we should be matching TB on this).
Within this bug, yes, it needs to match the current format.

Comment 26

[Ian Neal]

(In reply to rsx11m from comment #23)
> If you want to do it really sophisticated you'd get the current values of
> mailnews.reply_header_{authorwrote,colon,ondate,separator} and build those
> labels on the fly, but that may be overdoing it.
Possible but probably more effort than it is worth.

Comment 27

[rsx11m]

Agreed to comment #26, only an advanced user would tweak those preferences, thus the labels can reflect the defaults.

Comment 28

[Thomas D. (:thomas8)]

(In reply to Ian Neal from comment #25)
> > onDateWroteOption.label should really be "On [date], [author] wrote:", and
> > authorOnDateWroteOption.label should really be "[Author] wrote, On [date]:".

I assume this applies to English locale only (e.g. the first word order wouldn't work for German). The second proposal doesn't look good for English: Spelling: no capital after a comma, so at least, it should be "[Author] wrote, on [date]:" That's "Lisa wrote, on 2012-03-05 23:41:" I'm not sure if the comma is needed or good style. Perhaps we could do without: "Lisa wrote on 2012-03-05 23:41:"

Comment 29

[rsx11m]

Thomas, this bug cannot change the way how the MailNews backend works, it only provides a UI for it (and that's indeed what the backend comes up with).

Localizers can modify punctuation and wording as the language grammar requires it, so these cases are to that extent already considered.

Comment 30

[rsx11m]

Follow-up work is collected in bug 733258.

Comment 31

[Alexander L. Slovesnik]

Can anyone explain, should words inside brackets ([Author] and [date]) be translated?
I see that 3 locales (fr, nl and gl) left these words as in en-US, but es-AR locale have translated them as [Autor] and [fecha].

Comment 32

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Alexander L. Slovesnik from comment #31)
> Can anyone explain, should words inside brackets ([Author] and [date]) be
> translated?

Yes, they should. Think of these as variables.

> I see that 3 locales (fr, nl and gl) left these words as in en-US, but es-AR
> locale have translated them as [Autor] and [fecha].

Oh, sorry for that. I'll post to the l10n newsgroup then. In theory these languages could spell these words exactly like en-US, but I doubt it. ;-)

Please note that bug 733258 will tweak the exact wording of the options (and some of the entities) again (hopefully really soon now), as noted in this bug.

If you want to do the right thing for your locale, please open MailNews and reply to some message after changing the new UI preference (or mailnews.reply_header_type in about:config) to each of the four supported options (values 0-3).