fix formatting issues from bug #694514

RESOLVED FIXED in seamonkey2.10

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ewong, Assigned: ewong)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.10
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

Bug #694514 exposed mailnews.reply_header_type preference, but it needs some
minor fixes.
--
bug 694514 comment 28: (ThomasD)
> > 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:"

--
bug 694514 comment 25: (IanN)

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.

--
bug 694514 comment 22: (InvisibleSmiley)
::: 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]:".

---
bug 694514 comment 21: (InvisibleSmiley)

> +<!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.

--
bug 694514 comment 19: (sgautherie)

> +<!ENTITY onDateWroteOption.label              "On [date] [author] wrote:">

Care to s/onDateWroteOption/onDateAuthorWroteOption/g?
Blocks: 107884, 731223
> bug 694514 comment 28: (ThomasD)
> > > onDateWroteOption.label should really be "On [date], [author] wrote:", and
> > > authorOnDateWroteOption.label should really be "[Author] wrote, On [date]:".
> 
> "[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:"

That likely requires different mailnews.reply_header_ondate and .separator prefs to distinguish for case and punctuation with mailnews.reply_header_type=2 vs. =3, for which currently the same values are used. That's a separate mailnews bug, though.
Attachment #603140 - Flags: review?(iann_bugzilla)
Comment on attachment 603140 [details] [diff] [review]
Fixed issues from bug #694514. (v1)

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

BTW: back-end defaults for the prefs can be found here:
http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#134

::: 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 onDateAuthorWroteOption.label        "On [date], [author] wrote:">
> +<!ENTITY authorOnDateWroteOption.label        "[Author] wrote on [date]:">

Even though this does not match exactly what you will get by default, I think this (authorOnDateWroteOption.label) is OK now, because it's much closer to what an Englishman would like to have after tweaking some prefs. These can be found here:

http://mxr.mozilla.org/comm-central/source/mailnews/mailnews.js#211
http://mxr.mozilla.org/comm-central/source/suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties#321

As you can see from the above, separator and colon are defined in mailnews.js (i.e. code shared with TB, but could be overridden in browser-prefs.js), while authorwrote and ondate are defined in SM code land (l10n even).

Explaining all this in the UI is nonsense, but I think we should at least give some advice in Help which prefs affect this (which is why I just changed my r+ to r- over there).
Attachment #603140 - Attachment is obsolete: true
Attachment #603520 - Flags: review?(iann_bugzilla)
Attachment #603140 - Flags: review?(iann_bugzilla)
(In reply to Edmund Wong (:ewong) from comment #4)
> Created attachment 603520 [details] [diff] [review]
> Fix formatting issues from bug #694514. (v2)
Can you please add l10n notes for authorWroteOption.label, onDateAuthorWroteOption.label and authorOnDateWroteOption.label to clarify that [Author] and [date] should be translated? Looks like some localizers have some kind of confusion about it and left them as is, see Bug 694514 comment 32
(In reply to Alexander L. Slovesnik from comment #5)
> (In reply to Edmund Wong (:ewong) from comment #4)
> > Created attachment 603520 [details] [diff] [review]
> > Fix formatting issues from bug #694514. (v2)
> Can you please add l10n notes for authorWroteOption.label,
> onDateAuthorWroteOption.label and authorOnDateWroteOption.label to clarify
> that [Author] and [date] should be translated? Looks like some localizers
> have some kind of confusion about it and left them as is, see Bug 694514
> comment 32

Ah right, that reminds me that IMO we should also add a l10n note that these options depend on the mailnews.reply_header_* prefs.
Comment on attachment 603520 [details] [diff] [review]
Fix formatting issues from bug #694514. (v2)

Yes, need localiztion note explaining which prefs they are linked to and how they should be translated. r- so that I can review the note.
Attachment #603520 - Flags: review?(iann_bugzilla) → review-
Added localization notes to the dtd file.
Took the liberty of changing authorOnDateWroteOption to authorWroteOnDateOption
to better reflect what the option is.
Attachment #603520 - Attachment is obsolete: true
Attachment #603928 - Flags: review?(iann_bugzilla)
Comment on attachment 603928 [details] [diff] [review]
Fix formatting issues from bug #694514. (v3)

>+++ b/suite/locales/en-US/chrome/mailnews/pref/pref-composing_messages.dtd
>+<!-- LOCALIZATION NOTE (onDateAuthorWroteOption.label): this is tied to the combination of 
>+  mailnews.reply_header_ondate and mailnews.reply_header_authorwrote preferences. [date] and [Author] 
>+  need to be translated. -->
Should also mention where "," comes from too and how it is tied.

>+<!ENTITY onDateAuthorWroteOption.label        "On [date], [author] wrote:">
>+<!-- LOCALIZATION NOTE (authorWroteOnDateOption.label): this is tied to the mailnews.reply_header_authorwrote
>+  and mailnews.reply_header_ondate preferences. [Author] and [date] need to be translated. -->
Should also mention where "," comes from too and how it is tied.

>+<!ENTITY authorWroteOnDateOption.label        "[Author] wrote, On [date]:">
r=me with that fixed.
Attachment #603928 - Flags: review?(iann_bugzilla) → review+
Added also comment on separator.
Attachment #603928 - Attachment is obsolete: true
Attachment #604815 - Flags: review?(iann_bugzilla)
Hardware: x86 → All
Comment on attachment 604815 [details] [diff] [review]
Fix formatting issues from bug #694514. (v4)

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

>+<!ENTITY selectHeaderType.label               "Select reply header type:">
> <!ENTITY selectHeaderType.accesskey           "e">
> <!ENTITY noReplyOption.label                  "No Reply Header">
>+<!-- LOCALIZATION NOTE (authorWroteOption.label): this is tied to the mailnews.reply_header_authorwrote 
>+  preference. [Author] needs to be translated. -->
Nit: Sorry should have spotted this before, try and keep lines under 80 characters long.

> <!ENTITY authorWroteOption.label              "[Author] wrote:">
>+<!-- LOCALIZATION NOTE (onDateAuthorWroteOption.label): this is tied to the combination of 
>+  mailnews.reply_header_ondate, mailnews.reply_header_separator (the comma in this case) and 
>+  mailnews.reply_header_authorwrote preferences. [date] and [Author] need to be translated, and the correct 
>+  separator put in. -->
Nit: Try and keep lines under 80 characters long.

>+<!ENTITY onDateAuthorWroteOption.label        "On [date], [author] wrote:">
>+<!-- LOCALIZATION NOTE (authorWroteOnDateOption.label): this is tied to the mailnews.reply_header_authorwrote,
>+  mailnews.reply_header_separator (the comma in this case), and mailnews.reply_header_ondate preferences. 
>+  [Author] and [date] need to be translated, and the correct separator put in. -->
Nit: No "combination of" in this one, would be nice to be consistent across the two notes, so either have both with or both without. Try and keep lines under 80 characters long.
r=me with those fixed.
Attachment #604815 - Flags: review?(iann_bugzilla) → review+
Fixed nits.
Attachment #604815 - Attachment is obsolete: true
Attachment #604819 - Flags: review+
Attachment #604819 - Flags: review+
Attachment #604819 - Attachment is obsolete: true
Attachment #604822 - Flags: review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/89b9106c7db2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.10
You need to log in before you can comment on or make changes to this bug.