731223 - Update help for changes from bug #694514 and bug#733258.

Status: RESOLVED FIXED

(SeaMonkey :: Help Documentation, defect)

Description

[Edmund Wong (:ewong)]

Bug #694514 will have exposed mailnews.reply_header.  Update help to reflect these changes.

Comment 1

[Edmund Wong (:ewong)]

Attachment: Update help for changes from bug 694514. (v1)

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 602822 [details] [diff] [review]
Update help for changes from bug 694514. (v1)

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

r=me with the nit addressed *and* matching the UI (I strongly feel bug 694514 needs some tweaks, see Serge's and my comments there - better fix the UI first and then Help).

::: suite/locales/en-US/chrome/common/help/mailnews_preferences.xhtml
@@ +291,5 @@
> +      <ol><li>No Reply Header</li>
> +          <li>[Author] wrote:</li>
> +          <li>On [date] [author] wrote:</li>
> +          <li>[Author] on [date] wrote:</li>
> +      </ol></li>

Nit: The ordered list start and end tags should be on their own lines, with no additional indentation for themselves. So:
First level <li>, second level text and the ol start/end tags, third level the subordinate li items. The closing tag for the outer li item shall also be on its own line in this case (cf. line 90 in the same file).

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 602822 [details] [diff] [review]
Update help for changes from bug 694514. (v1)

Sorry, I changed my mind. Actually I'd like to see a new patch (once the exact wording of the options has been clarified) which shall also explain the relationship to the mailnews.reply_header_* prefs. Otherwise it won't be clear why there will be a capital "On" after the comma with option 4 (by default).

I'll put references to where the prefs can be found on bug 733258.

Comment 4

[Edmund Wong (:ewong)]

Attachment: Update help for changes from bug 694514. (v2)

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 604349 [details] [diff] [review]
Update help for changes from bug 694514. (v2)

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

r- since I want to see a new patch. Bear with me, we're almost there. :-)

::: suite/locales/en-US/chrome/common/help/mailnews_preferences.xhtml
@@ +285,5 @@
>      send the message when you're pressing Ctrl+Return in message editor.
>      This may help you avoid accidentally sending the message if you enter the
>      keyboard shortcut by mistake when composing a message.</li>
> +  <li><strong>Select reply header type</strong>:
> +    Select the type of reply header you wish to use.  There are four choices

Nit: Double space before "There".

@@ +292,5 @@
> +      <li>No Reply Header</li>
> +      <li>[Author] wrote:
> +        <ul>
> +          <li>This setting is based on the mailnews.reply_header_authorwrote preference.
> +          </li>

I don't see the benefit of an extra ul/li for the explanations (this one and the other two); a list only makes sense if it has at least two items. I think it would be better/best to just use a <br> after the option string instead.

@@ +299,5 @@
> +      <li>On [date], [author] wrote:
> +        <ul>
> +          <li>This setting is based on the mailnews.reply_header_ondate preference combined with
> +              the mailnews.reply_header_authorwrote preference separated by the mailnews.reply_header_separator
> +              preference option.

Nit (applied to this item and the next): I think "option" is redundant there, and "preference" used a little too often. Suggestion: "This setting is based on a combination of the X, Y and Z preferences." [I think the name of the third pref speaks for itself.]

Comment 6

[Edmund Wong (:ewong)]

Attachment: Update help for changes from bug 694514 and bug 733258 (v3)

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 604820 [details] [diff] [review]
Update help for changes from bug 694514 and bug 733258 (v3)

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

r=me with the nits fixed. Thanks!
Note: If you for some reason cannot check in this and/or bug 733258 *today*, please set checkin-needed accordingly since it needs to land ASAP (Aurora uplift = string freeze is *tomorrow*).

::: suite/locales/en-US/chrome/common/help/mailnews_preferences.xhtml
@@ +289,5 @@
> +    Select the type of reply header you wish to use. There are four choices
> +    available:
> +    <ol>
> +      <li>No Reply Header
> +      </li>

(Here and the other three times below:) The closing list item tag needs to be at the end of the line directly above. Basically the rule is: At the end of the line if the line contains text, with no space between the text and the closing tag.

@@ +292,5 @@
> +      <li>No Reply Header
> +      </li>
> +      <li>[Author] wrote:<br/>
> +         This setting is based on the mailnews.reply_header_authorwrote preference.
> +      </li>

Same as above, but if the line gets longer than 80 characters, please wrap accordingly.

@@ +296,5 @@
> +      </li>
> +      <li>On [date], [author] wrote:<br/>
> +         This setting is based on a combination of the mailnews.reply_header_ondate, 
> +         mailnews.reply_header_separator and mailnews.reply_header_authorwrote preferences.
> +      </li>

Same as above, but needs wrapping where possible (I think in this case before "preferences").

@@ +300,5 @@
> +      </li>
> +      <li>[Author] wrote, On [date]:<br/>
> +         This setting is based on a combination of the mailnews.reply_header_authorwrote,
> +         mailnews.reply_header_separator and mailnews.reply_header_ondate preferences.
> +      </li>

Same as above.

Comment 8

[Edmund Wong (:ewong)]

Attachment: Update help for changes from bug 694514 and bug 733258 (v4)

Fixed nits from comment #7.

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 604845 [details] [diff] [review]
Update help for changes from bug 694514 and bug 733258 (v4)

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

::: suite/locales/en-US/chrome/common/help/mailnews_preferences.xhtml
@@ +290,5 @@
> +    available:
> +    <ol>
> +      <li>No Reply Header</li>
> +      <li>[Author] wrote:<br/>
> +         This setting is based on the mailnews.reply_header_authorwrote 

(Bah, I'm very sorry, saw this before but forgot; here and below:) This should be one space less indent (2 spaces for each level).

Comment 10

[Edmund Wong (:ewong)]

Attachment: Update help for changes from bug 694514 and bug 733258 (part 2) (v1)

Fixed nits from previous patch.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 604850 [details] [diff] [review]
Update help for changes from bug 694514 and bug 733258 (part 2) (v1)

(Please land as a single patch.)

Comment 12

[Edmund Wong (:ewong)]

Pushed to comm-central:
(first r+ patch)
  http://hg.mozilla.org/comm-central/rev/2af54b6c02f2

(part 2)
  http://hg.mozilla.org/comm-central/rev/af896f514c15
  (thanks to a .hgrc screwup, it's attributed to builder. )

Comment 13

[Edmund Wong (:ewong)]

Backed out from comm-central: (due to author misattribution)
 http://hg.mozilla.org/comm-central/rev/988c0c1fd2f4

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