1247905 - Several string fixes in msgSecurityInfo.properties, both for mail and suite

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[Onno Ekker [:nONoNonO UTC+1]]

The string SINotYetValid in msgSecurityInfo.properties both in mail and in suite is missing the word "to":

Actual results:
SINotYetValid=The certificate used to sign the message appears not be valid yet. Make sure your computer's clock is set correctly.

Expected results:
SINotYetValid=The certificate used to sign the message appears not to be valid yet. Make sure your computer's clock is set correctly.

I don't think the entity name needs to change, since it's only a fix in the string and the string doesn't get a new meaning because of this change.

Comment 1

[Onno Ekker [:nONoNonO UTC+1]]

Attachment: SINotYetValid.patch

Comment 2

[Suyash Agarwal (:sshagarwal)]

Comment on attachment 8718795 [details] [diff] [review]
SINotYetValid.patch

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

I've added a few inline comments.

Thanks for taking this up.
Also, same comments apply to suite/ as well.

::: mail/locales/en-US/chrome/messenger-smime/msgSecurityInfo.properties
@@ +7,1 @@
>  SINone=This message does not include the sender's digital signature. The absence of a digital signature means that the message could have been sent by someone pretending to have this email address. It is also possible that the message has been altered while in transit over the network. However, it is unlikely that either event has occurred. 

Since we are here, can you please tweak a few other strings here as well?
There's this trailing space at the end of this string.

@@ +9,5 @@
>  SIValid=This message includes a valid digital signature. The message has not been altered since it was sent.
>  SIInvalidLabel=Digital Signature Is Not Valid
>  SIInvalidHeader=This message includes a digital signature, but the signature is invalid.
>  SIContentAltered=The signature does not match the message content correctly. The message appears to have been altered after the sender signed it. You should not trust the validity of this message until you verify its contents with the sender.
>  SIExpired=The certificate used to signed the message appears to have expired. Make sure your computer's clock is set correctly.

I think it should be "used to sign".

@@ +11,5 @@
>  SIInvalidHeader=This message includes a digital signature, but the signature is invalid.
>  SIContentAltered=The signature does not match the message content correctly. The message appears to have been altered after the sender signed it. You should not trust the validity of this message until you verify its contents with the sender.
>  SIExpired=The certificate used to signed the message appears to have expired. Make sure your computer's clock is set correctly.
>  SIRevoked=The certificate used to sign the message has been revoked. You should not trust the validity of this message until you verify its contents with the sender.
> +SINotYetValid=The certificate used to sign the message appears not to be valid yet. Make sure your computer's clock is set correctly.

Can we re-phrase this to "...used to sign the message is not yet valid..."?

@@ +19,5 @@
>  SIRevokedCA=The certificate used to sign the message was issued by a certificate authority whose own certificate has been revoked. You should not trust the validity of this message until you verify its contents with the sender.
>  SINotYetValidCA=The certificate used to sign the message was issued by a certificate authority whose own certificate is not yet valid. Make sure your computer's clock is set correctly.
>  SIInvalidCipher=The message was signed using an encryption strength that this version of your software does not support.
>  SIClueless=There are unknown problems with this digital signature. You should not trust the validity of this message until you verify its contents with the sender.
>  SIPartiallyValidLabel=Message is signed

I can't see the entire file here, but there's this string EINoneLabel which should be "Message is not encrypted".

Comment 3

[Onno Ekker [:nONoNonO UTC+1]]

Attachment: msgSecurityInfoProperties.patch_part1

Small fixes to strings in msgSecurityInfo.properties, both for mail and suite, that can be applied without needing to change the entities. This patch can maybe be applied to tb45? There's some more proposed changes coming up, that need to be localized too, so that patch should follow a different path.

Comment 4

[Philip Chee]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

r?IanN for the suite part

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

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

I'm stealing the review, so Magnus can do mine ;-) I can speak English, too.
I've checked the errors fixed:
was wrong: used to signed; now correct: used to sign
was wrong: appears not be valid; now correct: appears not to be valid
Trailing space removed.
So this is good: r+

Comment 6

[Onno Ekker [:nONoNonO UTC+1]]

So, what's next? Do I have to wait for review for SeaMonkey?
Or do I have to split the patch in a Thunderbird part and a SeaMonkey part, but then I need review for Thunderbird again too?
There's no flag for check-in, so what do I do to get this checked-in?
And can I set flag for approval '?' for comm-aurora and/or comm-beta?

Comment 7

[Jorg K (CEST = GMT+2)]

These are all very good questions. To me eye, fixing two grammar mistakes and removing a trailing space doesn't require another review by the suite people.

Personally, I'd just check it in and be done with it. But the SM people have their trees closed all the time and they have funny rules.

Let's ask Ratty/Philip.

On the issue of Aurora/beta: As far as I understand this, there are no string changes allowed. But then again, the translators most likely didn't translate the grammar errors. I'm not sure what the en-GB version does.

I'll ask Kent.

Summarising:
Ratty/Philip: Do you really need another review for these trivial changes or can this just be checked in, even if your trees are closed?
Kent: Can grammar corrections be uplifted?

Comment 8

[Philip Chee]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

(In reply to Jorg K (GMT+1) from comment #7)
> Personally, I'd just check it in and be done with it. But the SM people have
> their trees closed all the time and they have funny rules.

> Let's ask Ratty/Philip.
rs=me. a=me CLOSED TREE

> On the issue of Aurora/beta: As far as I understand this, there are no
> string changes allowed. But then again, the translators most likely didn't
> translate the grammar errors. I'm not sure what the en-GB version does.
IanN owns en-GB. Lets ask him.

> I'll ask Kent.
> 
> Summarising:
> Ratty/Philip: Do you really need another review for these trivial changes or
> can this just be checked in, even if your trees are closed?
rs=me. a=me CLOSED TREE

> Kent: Can grammar corrections be uplifted?

Comment 9

[Kent James (:rkent)]

On the issue of string uplifts, it would be better to ask the l10m people. I suppose the argument against it is the same argument that would say the original patch should not have been accepted without changing the string IDs, namely that bad grammar might have been translated incorrectly, but with no ID change this is never rechecked.

Comment 10

[Onno Ekker [:nONoNonO UTC+1]]

(In reply to Kent James (:rkent) from comment #9)
> On the issue of string uplifts, it would be better to ask the l10m people. I
> suppose the argument against it is the same argument that would say the
> original patch should not have been accepted without changing the string
> IDs, namely that bad grammar might have been translated incorrectly, but
> with no ID change this is never rechecked.

Fallen, can you confirm that no entity change is needed for these tpyo fixes?
And that it is safe to uplift?

Comment 11

[Rimas Kudelis [use needinfo to ping]]

It's quite unlikely that a localizer in their right mind would knowingly translate a string with such minor mistakes into a string with similar minor mistakes in their language. The meaning of these strings is not being changed in any way, just some typos are being fixed, thus there is no reason for the ID change. Nor is there a reason for not uplifting.
Just two cents from a localizer.

Comment 12

[Jorg K (CEST = GMT+2)]

OK, please check this in with
  r=jorgk rs=Ratty. a=Ratty CLOSED TREE
as per comment #8.

We'll worry about the uplift later.

Comment 13

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/08fba24fc221

Comment 14

[Jorg K (CEST = GMT+2)]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

[Approval Request Comment]
Regression caused by (bug #): No regression.
User impact if declined: Bad English will be displayed to the user.
Testing completed (on c-c, etc.): -
Risk to taking this patch (and alternatives if risky):
No risk.

However, looks like I was a little overzealous landing this without Fallen's OK for not changing the string ID. Ah well, I'll back it out if required.

Comment 15

[Onno Ekker [:nONoNonO UTC+1]]

I've cloned this bug to product Localization components en-GB and en-ZA, because there the same tpyos existed and without changing the entity name the localizers wouldn't be notified of the string change.

Comment 16

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

Feel free to ping me earlier if I don't respond to beta-relevant l10n requests. I don't see any entity changes here, thanks for also filing bugs for en-GB. You therefore have my approval to uplift this.

Comment 17

[Jorg K (CEST = GMT+2)]

Clearing NI for Ian since we are covered by bug 1249850.

Comment 18

[Jorg K (CEST = GMT+2)]

Aurora (TB 46, SM 2.43):
https://hg.mozilla.org/releases/comm-aurora/rev/7f809c571f77

Comment 19

[Jorg K (CEST = GMT+2)]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

This never landed on any beta and doesn't need to land on any beta. If anything, this needs to land on esr45 (which was at beta when beta got approved).

Comment 20

[Kent James (:rkent)]

Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

http://hg.mozilla.org/releases/comm-esr45/rev/2d02586c80c1