Closed Bug 694514 Opened 10 years ago Closed 10 years ago

Expose mailnews.reply_header_type preference

Categories

(SeaMonkey :: Preferences, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.10

People

(Reporter: iann_bugzilla, Assigned: ewong)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [mentor=IanN][lang=js])

Attachments

(1 file, 3 obsolete files)

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.
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.
I suggest:
// 4 - same as when we do a "Forward".
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.
Whiteboard: [mentor=IanN]
Whiteboard: [mentor=IanN] → [mentor=IanN][lang=js]
Blocks: 731223
Added to Mail & Newsgroups -> Composition.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #601552 - Flags: review?(iann_bugzilla)
Attachment #601552 - Attachment is obsolete: true
Attachment #601552 - Flags: review?(iann_bugzilla)
Attachment #601565 - Flags: review?(iann_bugzilla)
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.
Attachment #601565 - Flags: review?(iann_bugzilla) → review-
Attachment #601565 - Attachment is obsolete: true
Attachment #601899 - Flags: review?(iann_bugzilla)
Is it possible to add:
+              <menuitem value="5"
+                        label="&sameAsForwardOption.label;"/>
(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.
In fact - we were expecting to have &sameAsForwardOption.label;"/>
instead of one of the others ...
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.
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.
> 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.
Previous comment was directed to Edmund, comment #12 is off-topic.
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.
Attachment #601899 - Flags: review?(iann_bugzilla) → review+
Attachment #601899 - Attachment is obsolete: true
Attachment #602710 - Flags: review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/9f6c12aa2efb
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Edmund, can you attach screenshot of implemented SeaMonkey solution?

Is this something Thunderbird could/should do, too?
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?
Flags: in-testsuite-
Target Milestone: --- → seamonkey2.10
(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 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 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.
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.
(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.
(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.
(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.
Agreed to comment #26, only an advanced user would tweak those preferences, thus the labels can reflect the defaults.
(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:"
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.
Follow-up work is collected in bug 733258.
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].
(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).
Blocks: 13818
You need to log in before you can comment on or make changes to this bug.