588759 - Make sure status bar messages have proper punctuation

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Andreas Nilsson (:andreasn)]

Spinning off bug 79470 as an interesting discussion started there if we should use punctuation after the messages or not.
https://bugzilla.mozilla.org/show_bug.cgi?id=579470#c6

Comment 1

[Andreas Nilsson (:andreasn)]

According to Matthew Paul Thomas, when I spoke to him on IRC, we should not use full stops in those situations.

<mpt_> andreasn, when I worked on status bar messages for Mozilla (which were inherited by Firefox), I specified that temporary or progress messages should end in ellipsis (e.g. "Waiting for bugzilla.mozilla.org…"), while others should not end in any punctuation (e.g. "Done")

Comment 2

[Magnus Melin [:mkmelin]]

One sample - http://hg.mozilla.org/comm-central/annotate/6c605636afe1/mail/locales/en-US/chrome/messenger/imapMsgs.properties#l217

(though that file is i believe being changed away from number based strings)

Probably a lot of investigation for someone taking this on to find and check which things are statues... Also note you have to change the localization key when changing the value, otherwise localizers can't keep up.

Comment 3

[Reese Chappuis]

Hey all- New to the community, trying to get in on development. I figure this bug is fairly straightforward, something to get me familiar with some of the directory structure. So... I'd be happy to take this on.

Comment 4

[Reese Chappuis]

Quick question: if a message has multiple sentences, should I still take off the final period? Personally, it feels a little unbalanced in that case.

Comment 5

[Magnus Melin [:mkmelin]]

Yes, for multiple sentences there should probably always be a final period.

Comment 6

[Andre Baptiste]

I'm having a little trouble finding where the text for the status bar is in the code. Could someone tell me where I should look?

Comment 7

[Andre Baptiste]

Sorry to double post. I had meant to ask if the sample given by Magnus Melin was the only portion that needed editing. I would like to take the time to consult with someone over each line and it would be better if I gathered as many of the messages as possible beforehand.

Comment 8

[Andre Baptiste]

Attachment: statuses.patch changes the formatting of status messages and their localization keys

I had some difficulty learning to use Mercurial and the other tools. Because of that, it is likely that the patch itself is formatted incorrectly. Handle with caution.

Comment 9

[Magnus Melin [:mkmelin]]

The patch format looks good, but preferably use an 8 line context for easier reviews
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F 

... 

[diff]
git = 1
showfunc = 1
unified = 8

Also, the patch comment is usually the bug summary.

After you fix those minor issues, please submit the patch for review by setting the "review?" flag to suitable reviewers.
(Me, maybe [e-mail], and ui-review from [e-mail])

https://developer.mozilla.org/docs/En/Developer_Guide/How_to_Submit_a_Patch#Getting_the_patch_reviewed

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 8491885 [details] [diff] [review]
statuses.patch changes the formatting of status messages and their localization keys

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

::: mail/locales/en-US/chrome/messenger/imapMsgs.properties
@@ +202,4 @@
>  # Place the word %3$S in your translation where the name of the destination folder should appear.
>  # Place the word %1$S where the currently copying message should appear.
>  # Place the word %2$S where the total number of messages should appear.
> +imapCopyingMessageOf2=Copying Message %1$S of %2$S to %3$S…

we should make this Message lower cased while we're touching this

Comment 11

[Andre Baptiste]

Attachment: status.diff changes status message formatting and localization keys

Some status messages in imapMsgs.properties under mail used incorrect punctuation.

Comment 12

[Ian Neal]

Comment on attachment 8492561 [details] [diff] [review]
status.diff changes status message formatting and localization keys


>-# LOCALIZATION NOTE (imapCopyingMessageOf): Do not translate the word "%S" below.
>+# LOCALIZATION NOTE (imapCopyingMessageOf2): Do not translate the word "%S" below.
> # Place the word %3$S in your translation where the name of the destination folder should appear.
> # Place the word %1$S where the currently copying message should appear.
> # Place the word %2$S where the total number of messages should appear.
>-imapCopyingMessageOf=Copying Message %1$S of %2$S to %3$S
>+imapCopyingMessageOf2=Copying Message %1$S of %2$S to %3$Sâ¦

You missed changing Message to message here.

Comment 13

[Andre Baptiste]

Yes, it shows up twice and I only noticed the first. Is there a way to replace or edit an attachment or do I just have to post it again?(In reply to Ian Neal from comment #12)
> Comment on attachment 8492561 [details] [diff] [review]
> status.diff changes status message formatting and localization keys
> 
> 
> >-# LOCALIZATION NOTE (imapCopyingMessageOf): Do not translate the word "%S" below.
> >+# LOCALIZATION NOTE (imapCopyingMessageOf2): Do not translate the word "%S" below.
> > # Place the word %3$S in your translation where the name of the destination folder should appear.
> > # Place the word %1$S where the currently copying message should appear.
> > # Place the word %2$S where the total number of messages should appear.
> >-imapCopyingMessageOf=Copying Message %1$S of %2$S to %3$S
> >+imapCopyingMessageOf2=Copying Message %1$S of %2$S to %3$Sâ¦
> 
> You missed changing Message to message here.

Comment 14

[Josiah Bruner [:jsbruner]]

(In reply to Andre Baptiste from comment #13)
> Yes, it shows up twice and I only noticed the first. Is there a way to
> replace or edit an attachment or do I just have to post it again?(In reply
> to Ian Neal from comment #12)
> > Comment on attachment 8492561 [details] [diff] [review]
> > status.diff changes status message formatting and localization keys
> > 
> > 
> > >-# LOCALIZATION NOTE (imapCopyingMessageOf): Do not translate the word "%S" below.
> > >+# LOCALIZATION NOTE (imapCopyingMessageOf2): Do not translate the word "%S" below.
> > > # Place the word %3$S in your translation where the name of the destination folder should appear.
> > > # Place the word %1$S where the currently copying message should appear.
> > > # Place the word %2$S where the total number of messages should appear.
> > >-imapCopyingMessageOf=Copying Message %1$S of %2$S to %3$S
> > >+imapCopyingMessageOf2=Copying Message %1$S of %2$S to %3$Sâ¦
> > 
> > You missed changing Message to message here.

You can't modify the bugzilla attachment, you will have to upload a new one. Now if you understand HG decently well, you need not change your source code, but instead can modify your existing patch file.

Comment 15

[Andre Baptiste]

Attachment: status.diff changes the formatting of status messages and their localization keys

Some status messages in imapMsgs.properties under mail used incorrect end punctuation and capitalization.

Comment 16

[Magnus Melin [:mkmelin]]

Comment on attachment 8492822 [details] [diff] [review]
status.diff changes the formatting of status messages and their localization keys

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

It appears all the ellipsis are now corrupted. Please ensure everything is UTF-8

Comment 17

[Ian Neal]

Comment on attachment 8492822 [details] [diff] [review]
status.diff changes the formatting of status messages and their localization keys

>+++ b/mail/locales/en-US/chrome/messenger/imapMsgs.properties

>+# LOCALIZATION NOTE (imapReceivingMessageHeaders2): Do not translate the word "%1$S", "%2$lu" or "%3$lu" below.
> # Place the word %1$S in your translation where the name of the server should appear.
> # Place the word %2$lu where the number of the header currently being downloaded should appear.
> # Place the word %3$lu where the number of headers should appear.
These comments no longer match what is below. They appear to be correct in suite/ equivalent though.
>+imapReceivingMessageHeaders2=%S Downloading message header %lu of %luâ¦


>+# LOCALIZATION NOTE (imapReceivingMessageFlags2): Do not translate the word "%1$S", "%2$lu" or "%3$lu" below.
> # Place the word %1$S in your translation where the name of the server should appear.
> # Place the word %2$lu where the number of the flag currently being downloaded should appear.
> # Place the word %3$lu where the number of flags should appear.
These comments no longer match what is below. They appear to be correct in suite/ equivalent though.
>+imapReceivingMessageFlags2=%S Downloading message flag %lu of %luâ¦

Comment 18

[Josiah Bruner [:jsbruner]]

Comment on attachment 8492822 [details] [diff] [review]
status.diff changes the formatting of status messages and their localization keys

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

I don't know why you're adding '2' to everything, and the fact that this is not in UTF-8 means ui-r- for now.

Comment 19

[Magnus Melin [:mkmelin]]

Andre: any update?

Comment 20

[Andre Baptiste]

(In reply to Magnus Melin from comment #19)
> Andre: any update?

I'm afraid not. I've gotten caught up in some school project so I don't know when I'll come back to this. I'll try again once I have more time but until then I should probably be taken off.

Comment 21

[Magnus Melin [:mkmelin]]

Attachment: 588759.patch

Updated the patch, so we can hopefully get this landed. r=me for the mail/ part

Josiah: to answer your question in comment 18. We have to update the localization key, so localizers notice the change.

Comment 22

[Josiah Bruner [:jsbruner]]

Comment on attachment 8543184 [details] [diff] [review]
588759.patch

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

::: mail/locales/en-US/chrome/messenger/imapMsgs.properties
@@ +77,5 @@
>  
> +# LOCALIZATION NOTE (imapReceivingMessageHeaders2): Do not translate the word "%S" or "%lu" below.
> +# Place the word %S in your translation where the name of the server should appear.
> +# Place the word %lu where the number of headers should appear.
> +imapReceivingMessageHeaders2=%S Downloading message header %lu of %luâ¦

I think the encoding got messed up (Happens here and many other places).

::: suite/locales/en-US/chrome/mailnews/imapMsgs.properties
@@ +84,3 @@
>  # Place the word %S in your translation where the name of the server should appear.
>  # Place the word %lu where the number of headers should appear.
> +imapReceivingMessageHeaders2=%S Downloading message header %lu of %luâ¦

I think the encoding got messed up (Happens here and many other places).

Comment 23

[Magnus Melin [:mkmelin]]

It appears splinter mangles the ellipsis. If you look at it raw, https://bug588759.bugzilla.mozilla.org/attachment.cgi?id=8543184 its correctly UTF-8

Comment 24

[Josiah Bruner [:jsbruner]]

Comment on attachment 8543184 [details] [diff] [review]
588759.patch

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

Ah, alright. Looks fine then.

Comment 25

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/d2889360a981 -> FIXED

Apparently there's a bug in hg that made the UTF-8 messed up for display in the patch. The problem seems to be that an ellipsis is on the hunk marker row (@@ -102,20 +100,20 @@ ) and THAT is wrongly encoded, though it's correct in the actual file.