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

RESOLVED FIXED in Thunderbird 47.0

Status

--
trivial
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: nONoNonO, Assigned: nONoNonO)

Tracking

unspecified
Thunderbird 47.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird46 fixed, thunderbird47 fixed, thunderbird_esr4547+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

Comment 1

3 years ago
Created attachment 8718795 [details] [diff] [review]
SINotYetValid.patch
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".
Attachment #8718795 - Flags: feedback+
Assignee: nobody → o.e.ekker
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Summary: Missing word "to" in string SINotYetValid → Several string fixes in msgSecurityInfo.properties, both for mail and suite
(Assignee)

Comment 3

3 years ago
Created attachment 8718902 [details] [diff] [review]
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.
Attachment #8718795 - Attachment is obsolete: true
Attachment #8718902 - Flags: review?(mkmelin+mozilla)

Updated

3 years ago
Attachment #8718902 - Attachment is patch: true

Comment 4

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

r?IanN for the suite part
Attachment #8718902 - Flags: review?(iann_bugzilla)

Comment 5

3 years ago
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+
Attachment #8718902 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 6

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

3 years ago
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?
Flags: needinfo?(rkent)
Flags: needinfo?(philip.chee)

Comment 8

3 years ago
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?
Flags: needinfo?(philip.chee) → needinfo?(iann_bugzilla)
Attachment #8718902 - Flags: review?(iann_bugzilla) → review+
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.
Flags: needinfo?(rkent)
(Assignee)

Comment 10

3 years ago
(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?
Flags: needinfo?(philipp)

Comment 11

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

3 years ago
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.
Keywords: checkin-needed

Comment 13

3 years ago
https://hg.mozilla.org/comm-central/rev/08fba24fc221
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-thunderbird45: --- → affected
status-thunderbird46: --- → affected
status-thunderbird47: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 47.0

Comment 14

3 years ago
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.
Attachment #8718902 - Flags: approval-comm-beta?
Attachment #8718902 - Flags: approval-comm-aurora+
(Assignee)

Updated

3 years ago
Blocks: 1249850
(Assignee)

Updated

3 years ago
Blocks: 1249851
(Assignee)

Comment 15

3 years ago
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 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.
Flags: needinfo?(philipp)
Attachment #8718902 - Flags: approval-comm-beta? → approval-comm-beta+

Comment 17

3 years ago
Clearing NI for Ian since we are covered by bug 1249850.
Flags: needinfo?(iann_bugzilla)

Comment 18

3 years ago
Aurora (TB 46, SM 2.43):
https://hg.mozilla.org/releases/comm-aurora/rev/7f809c571f77
status-thunderbird46: affected → fixed
(Assignee)

Updated

3 years ago
Blocks: 1249865

Comment 19

3 years ago
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).
Attachment #8718902 - Flags: approval-comm-beta+ → approval-comm-esr45?

Updated

3 years ago
status-thunderbird45: affected → ---
status-thunderbird_esr45: --- → affected
tracking-thunderbird_esr45: --- → ?
Comment on attachment 8718902 [details] [diff] [review]
msgSecurityInfoProperties.patch_part1

http://hg.mozilla.org/releases/comm-esr45/rev/2d02586c80c1
Attachment #8718902 - Flags: approval-comm-esr45? → approval-comm-esr45+

Updated

3 years ago
status-thunderbird_esr45: affected → fixed
tracking-thunderbird_esr45: ? → 47+
You need to log in before you can comment on or make changes to this bug.