Expose mailnews.reply_header_type preference

RESOLVED FIXED in seamonkey2.10

Status

SeaMonkey
Preferences
--
enhancement
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ian Neal, Assigned: ewong)

Tracking

(Blocks: 2 bugs)

Trunk
seamonkey2.10
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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

6 years ago
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

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

Comment 3

6 years ago
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.
(Assignee)

Updated

6 years ago
Whiteboard: [mentor=IanN]
(Assignee)

Updated

6 years ago
Whiteboard: [mentor=IanN] → [mentor=IanN][lang=js]
(Assignee)

Updated

6 years ago
Blocks: 731223
(Assignee)

Comment 4

6 years ago
Created attachment 601552 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v1)

Added to Mail & Newsgroups -> Composition.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #601552 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 5

6 years ago
Created attachment 601565 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v2)
Attachment #601552 - Attachment is obsolete: true
Attachment #601552 - Flags: review?(iann_bugzilla)
Attachment #601565 - Flags: review?(iann_bugzilla)
(Reporter)

Comment 6

6 years ago
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-
(Assignee)

Comment 7

6 years ago
Created attachment 601899 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v3)
Attachment #601565 - Attachment is obsolete: true
Attachment #601899 - Flags: review?(iann_bugzilla)

Comment 8

6 years ago
Is it possible to add:
+              <menuitem value="5"
+                        label="&sameAsForwardOption.label;"/>
(Reporter)

Comment 9

6 years ago
(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

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

Comment 11

6 years ago
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

6 years ago
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

6 years ago
> 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

6 years ago
Previous comment was directed to Edmund, comment #12 is off-topic.
(Reporter)

Comment 15

6 years ago
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+
(Assignee)

Comment 16

6 years ago
Created attachment 602710 [details] [diff] [review]
Expose mailnews.reply_header_type preference. (v4)
Attachment #601899 - Attachment is obsolete: true
Attachment #602710 - Flags: review+
(Assignee)

Comment 17

6 years ago
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/9f6c12aa2efb
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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

Comment 20

6 years ago
(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.

Comment 23

6 years ago
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.
(Reporter)

Comment 24

6 years ago
(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.
(Reporter)

Comment 25

6 years ago
(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.
(Reporter)

Comment 26

6 years ago
(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

6 years ago
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:"

Comment 29

6 years ago
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

6 years ago
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).

Updated

6 years ago
Blocks: 13818
You need to log in before you can comment on or make changes to this bug.