convert openpgp strings to Fluent, move to content
Categories
(MailNews Core :: Security: OpenPGP, task, P1)
Tracking
(thunderbird_esr78 fixed)
| Tracking | Status | |
|---|---|---|
| thunderbird_esr78 | --- | fixed |
People
(Reporter: mkmelin, Assigned: khushil324)
References
Details
Attachments
(16 files, 30 obsolete files)
We have a lot of (enigmail) strings that are currently in content/ so we can change them freely to iterate.
These strings should be moved to a localizable place, but first they should be converted to using Fluent so that we don't have to go through a tedious migration process for them.
I'm talking about (at least) these:
https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/strings
https://firefox-source-docs.mozilla.org/l10n/fluent/tutorial.html
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 3•4 years ago
|
||
Comment on attachment 9151269 [details] [diff] [review] Bug-1638822_openpgp-fluent-bond-0.patch Review of attachment 9151269 [details] [diff] [review]: ----------------------------------------------------------------- Can we lose the enigmail branding while we're doing this. So instead of enigmail- name them openpgp- ::: mail/locales/en-US/messenger/openpgp/bond.ftl @@ +3,5 @@ > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +enigmail-manage-keys-openpgp-cmd = > + .label = OpenPGP Key Management… > + .accesskey = o accesskeys are case sensitive (but fall back to working if right case is not found. @@ +5,5 @@ > +enigmail-manage-keys-openpgp-cmd = > + .label = OpenPGP Key Management… > + .accesskey = o > + > +enigmail-ctx-decryptOpen = enigmail-ctx-decrypt-open
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 4•4 years ago
|
||
Comment on attachment 9151270 [details] [diff] [review] Bug-1638822_openpgp-fluent-composeKeyStatus-0.patch Review of attachment 9151270 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good, but change the enigmail- to openpgp- Also, please keep the flt file in content for now, and then we move it to the right place later.
|
Assignee | |
Comment 5•4 years ago
|
||
|
|
Assignee | |
Comment 6•4 years ago
|
||
|
|
Assignee | |
Comment 7•4 years ago
|
||
|
|
Reporter | |
Comment 8•4 years ago
|
||
Comment on attachment 9151434 [details] [diff] [review] Bug-1638822_openpgp-fluent-bond-1.patch Review of attachment 9151434 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/base/content/mainPopupSet.inc.xhtml @@ +28,5 @@ > accesskey="&CopyNameAndEmailAddress.accesskey;" > oncommand="CopyEmailNewsAddress(document.popupNode, true)"/> > #ifdef MOZ_OPENPGP > <menuseparator/> > + <menuitem id="searchKeysOpenPGP" data-l10n-id="openpgp-search-keys-openpgp" this one doesn't seem to work (the contact popup menu to Disover Key) maybe the Fluent file is not there... ::: mail/base/content/msgOpenPGPKey.inc.xhtml @@ +7,3 @@ > <toolbarbutton id="openpgpImportButton" > + data-l10n-id="openpgp-import-sender-key" > + oncommand="Enigmail.msg.importAutocryptKeydata();"/> let's fix indention of the toolbarbutton while we're touching this ::: mail/extensions/openpgp/content/strings/bond.ftl @@ +3,5 @@ > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +openpgp-manage-keys-openpgp-cmd = > + .label = OpenPGP Key Management… > + .accesskey = o access keys are case sensitive (but fall back to working) @@ +9,5 @@ > +openpgp-ctx-decrypt-open = > + .label = Decrypt and Open > + .accesskey = D > +openpgp-ctx-decrypt-save = > + .label = Decrypt and Save As... should be ellipsis
|
|
Reporter | |
Comment 9•4 years ago
|
||
Comment on attachment 9151435 [details] [diff] [review] Bug-1638822_openpgp-fluent-composeKeyStatus-msgReadStatus-0.patch Review of attachment 9151435 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/ui/msgReadStatus.js @@ +109,5 @@ > + document.getElementById("signatureLabel"), > + sigInfoLabel > + ); > + if (hasAnySig) { > + setText("signatureExplanation", sigInfo, true); I think it would be better to, as you transition this to fluent, not use the setText helper but type out the full call (like the rows above)
|
|
Reporter | |
Comment 10•4 years ago
|
||
Comment on attachment 9151437 [details] [diff] [review] Bug-1638822_openpgp-fluent-oneRecipientStatus-0.patch Review of attachment 9151437 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, r=mkmelin
|
|
Assignee | |
Comment 11•4 years ago
|
||
|
|
Assignee | |
Comment 12•4 years ago
|
||
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 13•4 years ago
|
||
I'll land these so we don't bitrot.
|
||
Comment 14•4 years ago
|
||
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/ea9f35cac5c3 convert openpgp strings to Fluent - bond.ftl. r=mkmelin https://hg.mozilla.org/comm-central/rev/c931a1446435 convert openpgp strings to Fluent - composeKeyStatus.ftl and msgReadStatus.ftl. r=mkmelin https://hg.mozilla.org/comm-central/rev/45df638cd2b2 convert openpgp strings to Fluent - oneRecipientStatus.ftl. r=mkmelin
|
|
Assignee | |
Comment 15•4 years ago
|
||
|
||
Comment 16•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 17•4 years ago
|
||
There is a functional change in msgReadStatus, for the upper signature status. The short summary header is done. Now the longer explanation text is shown as bold.
|
|
||
Comment 18•4 years ago
|
||
|
|
||
Comment 19•4 years ago
|
||
|
|
Assignee | |
Comment 20•4 years ago
|
||
Comment on attachment 9152360 [details] [diff] [review] 1638822-fix-read-sig-explanation-v2.patch Review of attachment 9152360 [details] [diff] [review]: ----------------------------------------------------------------- r=khushil. Thanks for the changes.
|
|
Assignee | |
Comment 21•4 years ago
|
||
Check-in needed for 1638822-fix-openpgp-one-recipient-status-status.patch and 1638822-fix-read-sig-explanation-v2.patch.
|
|
Reporter | |
Comment 22•4 years ago
|
||
Comment on attachment 9152146 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-dtd-0.patch Review of attachment 9152146 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/strings/enigmail.ftl @@ +42,5 @@ > +openpgp-key-man-gen-revoke = > + .label = Revocation Certificate > + .accesskey = R > +openpgp-key-man-ctx-gen-revoke-label = > + .label = Generate & Save Revocation Certificate with Fluent, can't it just be & ? @@ +161,5 @@ > + .label = Size > +openpgp-key-details-created-label = Created > + .label = Created > +openpgp-key-details-expiry-label = Expiry > + .label = Expiry these too ^^ - are they correct?
|
|
||
Comment 23•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/28d70f3af773
fix for upper signature status. r=khushil
https://hg.mozilla.org/comm-central/rev/ec0e99a2f1e4
fix read sig explanation. r=khushil
|
|
Reporter | |
Comment 24•4 years ago
|
||
Kai - please set up your .hg username/email. (I noticed to late, it's wrong here.)
|
|
Assignee | |
Comment 25•4 years ago
|
||
|
|
Reporter | |
Comment 26•4 years ago
|
||
Comment on attachment 9152487 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-dtd-1.patch Review of attachment 9152487 [details] [diff] [review]: ----------------------------------------------------------------- Tools | Open PGP key management dialog is pretty busted with this. No title. And if I click open a key the information dialog is quite incomplete. I get this: console.warn: "[fluent] Request for keys failed because no resource bundles got generated.\n keys: [{\"args\":null,\"id\":\"openpgp-key-details-title\"},{\"args\":null,\"id\":\"openpgp-card-details-close-window-label\"},{\"args\":null,\"id\":\"openpgp-key-details-user-id2-label\"},{\"args\":null,\"id\":\"openpgp-key-details-key-type-label\"},{\"args\":null,\"id\":\"openpgp-key-details-fingerprint-label\"},{\"args\":null,\"id\":\"openpgp-key-details-created-header\"},{\"args\":null,\"id\":\"openpgp-key-details-expiry-header\"},{\"args\":null,\"id\":\"openpgp-key-details-also-known-label\"},{\"args\":null,\"id\":\"openpgp-acceptance-label\"},{\"args\":null,\"id\":\"openpgp-key-details-signatures-tab\"},{\"args\":null,\"id\":\"openpgp-key-details-structure-tab\"},{\"args\":null,\"id\":\"openpgp-acceptance-rejected-label\"},{\"args\":null,\"id\":\"openpgp-acceptance-undecided-label\"},{\"args\":null,\"id\":\"openpgp-acceptance-unverified-label\"},{\"args\":null,\"id\":\"openpgp-acceptance-verified-label\"},{\"args\":null,\"id\":\"openpgp-key-details-uid-certified-col\"},{\"args\":null,\"id\":\"openpgp-key-id-label\"},{\"args\":null,\"id\":\"openpgp-key-details-created-label\"},{\"args\":null,\"id\":\"openpgp-key-details-key-part-label\"},{\"args\":null,\"id\":\"openpgp-key-details-usage-label\"},{\"args\":null,\"id\":\"openpgp-key-details-id-label\"},{\"args\":null,\"id\":\"openpgp-key-details-algorithm-label\"},{\"args\":null,\"id\":\"openpgp-key-details-algorithm-label\"},{\"args\":null,\"id\":\"openpgp-key-details-created-label\"},{\"args\":null,\"id\":\"openpgp-key-details-expiry-label\"}].\n resourceIds: [\"messenger/openpgp/enigmail.ftl\"]."
|
|
Reporter | |
Comment 27•4 years ago
|
||
Sorry, I had forgot to rebuild!
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 28•4 years ago
|
||
Comment on attachment 9152487 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-dtd-1.patch Review of attachment 9152487 [details] [diff] [review]: ----------------------------------------------------------------- Besides the thing mentioned below, I didn't find any problems. ::: mail/extensions/openpgp/content/ui/keyDetailsDlg.xhtml @@ +157,4 @@ > persist="width"/> > <splitter class="tree-splitter"/> > <treecol id="sizeCol" style="width:37px" > + data-l10n-id="openpgp-key-details-algorithm-label" This one is openpgp-key-details-size-label
|
|
||
Comment 29•4 years ago
|
||
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/4367af3d5a4a convert openpgp strings to Fluent - enigmail.ftl. r=mkmelin
|
|
Reporter | |
Comment 30•4 years ago
|
||
Some minor adjustments for strings, while we can.
|
|
||
Comment 31•4 years ago
|
||
Typo:
.label = Display Untrusted Ueys
This is missing the word "it"
.label = Yes, but I have not verified that is the correct key.
It should be
.label = Yes, but I have not verified that it is the correct key.
But I'm not sure I like your change.
You removed the suffixes (rejected), (undecided), (unverified) and (verified) from the acceptance selection.
I had added those labels as a one-word summary, to enable a recognition effect in other places.
In the one receipient dialog you kept those labels.
|
|
||
Comment 32•4 years ago
•
|
||
To clarify my comment. I'd prefer to keep the trailing "(...)" elements in the following strings:
openpgp-acceptance-rejected-label =
- .label = No, never use this key. (rejected)
openpgp-acceptance-undecided-label =
- .label = Not yet, maybe later. (undecided)
openpgp-acceptance-unverified-label =
- .label = Yes, but I don't know if it is the correct key. (unverified)
openpgp-acceptance-verified-label =
- .label = Yes, I've verified in person this key has the correct fingerprint. (verified)
|
|
Reporter | |
Comment 33•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #31)
But I'm not sure I like your change.
You removed the suffixes (rejected), (undecided), (unverified) and (verified) from the acceptance selection.
Yes, it's super ugly.
I had added those labels as a one-word summary, to enable a recognition effect in other places.
In the one receipient dialog you kept those labels.
Correct. Since it's a column I'm not sure we can do better. I'm not too thrilled about that UI either but have no easy suggestions.
|
|
Reporter | |
Comment 34•4 years ago
|
||
|
|
||
Updated•4 years ago
|
|
|
||
Comment 35•4 years ago
|
||
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/a1b05e170dd6 minor string fixes for openpgp. r=kaie DONTBUILD
|
|
Assignee | |
Comment 36•4 years ago
|
||
|
|
Reporter | |
Comment 37•4 years ago
|
||
Comment on attachment 9153346 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-0.patch Review of attachment 9153346 [details] [diff] [review]: ----------------------------------------------------------------- I think we'll need to land the strings we have. For finding the unused things - could be easier to put e.g. ### for those lines instead so that they stand out (and can be grepped out) ::: mail/extensions/openpgp/content/strings/enigmail.ftl @@ +200,5 @@ > +key-usage-authentication = Authentication > +key-does-not-expire = Key does not expire > +key-expired = Key expired on { $keyExpiry } > +key-revoked = Key was revoked > +key-auto-accept-personal = You accept this key for all uses, because it is one of your personal keys. (You have the secret key.) Let's remove the parenthesis ::: mail/extensions/openpgp/content/ui/keyDetailsDlg.js @@ +61,5 @@ > > +function setl10nLabel(elementId, label, argument) { > + let node = document.getElementById(elementId); > + document.l10n.setAttributes(node, label); > +} I think you don't use this helper, and it's probably clearer not having one too.
|
|
Reporter | |
Comment 38•4 years ago
|
||
This is a patch I had to move to a localizable location. Maybe you want to incorporate it?
|
|
Reporter | |
Comment 39•4 years ago
|
||
Or let's hold that, for now.
Let's keep the strings non-localizable for now until they stabilize.
|
|
Assignee | |
Comment 40•4 years ago
|
||
|
|
Assignee | |
Comment 41•4 years ago
|
||
|
|
Assignee | |
Comment 42•4 years ago
|
||
|
|
Assignee | |
Comment 43•4 years ago
|
||
|
|
Assignee | |
Comment 44•4 years ago
|
||
Kai, can you also look at these patches and if you find any problems, can you describe steps to reproduce them?
|
|
||
Comment 45•4 years ago
|
||
If you use a single bug for longer, can you please mark patches, which are already done, using some indicator? You could edit the description of the patch and add [checked in] at the end. That would make it easier for me to know which patches are still left undone, and to which patches you're referring to in your request.
Ideally, if you work on changes in these area, you should try the related UI of all affected code yourself, and not depend on me to discover failures. Could you setup OpenPGP for yourself in a test profile and experiment with it?
|
|
Assignee | |
Comment 46•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #45)
Ideally, if you work on changes in these area, you should try the related UI of all affected code yourself, and not depend on me to discover failures. Could you setup OpenPGP for yourself in a test profile and experiment with it?
Yes, I am doing this. But here we are working with 200 odd strings and all these strings are inserted through Javascript and JSM files. And it's pretty much covering all the functionalities. So it's likely that I can miss a few things and we don't want to break things right now.
|
|
||
Comment 47•4 years ago
|
||
Thanks for the feedback, I'm glad you're testing, too.
Can you please say which of the many patches I should apply for testing?
|
|
Assignee | |
Comment 48•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #47)
Thanks for the feedback, I'm glad you're testing, too.
Can you please say which of the many patches I should apply for testing?
Patch for which I have asked Magnus's review. I have tried it to slipt into multiple patches. There are currently 4 patches. 2 more patches are coming. Name: Bug-1638822_openpgp-fluent-enigmail-properties-part-1-to-part-6.patch
I am going to test them once again tomorrow as now I have converted all the strings. You can look at them after I do the final testing.
|
|
Assignee | |
Comment 49•4 years ago
|
||
|
|
Assignee | |
Comment 50•4 years ago
|
||
|
|
||
Comment 51•4 years ago
|
||
tools / openpgp key manager / file / import public
fails with
JavaScript error: resource://gre/modules/Localization.jsm, line 347: Error: Can't use sync formatWithFallback when state is async.
JavaScript error: chrome://openpgp/content/ui/commonWorkflows.js, line 58: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "Can't use sync formatWithFallback when state is async." {file: "resource://gre/modules/Localization.jsm" line: 347}]'[JavaScript Error: "Can't use sync formatWithFallback when state is async." {file: "resource://gre/modules/Localization.jsm" line: 347}]' when calling method: [mozILocalization::formatValueSync]
|
|
||
Comment 52•4 years ago
•
|
||
additional problem:
to reproduce, ensure you have a personal openpgp key configured already.
to check that you have, use tools / openpgp key manager and confirm you have a line that is bold. If you double click it, it should say "type personal key".
If you don't have one yet, use menu generate to create a new one for you
now that you have this prepared, use the following to reproduce the failure:
go to "edit / account settings / end-to-end encryption"
in the "openpgp" section, click button "set personal key".
a dialog will open.
Expected behavior:
the list shows your personal keys
Actual behavior with your patches applied:
the list is empty
|
|
||
Comment 53•4 years ago
|
||
I'll stop testing for now. Please let me know once you have these failures fixed, then I can do more testing.
|
|
||
Comment 54•4 years ago
|
||
Note that the test results from comment 51 and 52 were done using a comm-beta 78 branch build.
I tried a m-c 79 build, but that doesn't even start up. I get:
JavaScript error: chrome://openpgp/content/modules/trust.jsm, line 15: TypeError: Localization is not a constructor
which prevents loading of the main messenger window.
|
|
Assignee | |
Comment 55•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 56•4 years ago
|
||
|
|
Assignee | |
Comment 57•4 years ago
|
||
|
|
Assignee | |
Comment 58•4 years ago
|
||
Needed a rebase. I have checked almost every functionality now. Let's merge these 3 patches first so that it's easy to find the problem. I have checked this on the Trunk.
|
|
Reporter | |
Comment 59•4 years ago
|
||
Comment on attachment 9154582 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-2.patch Review of attachment 9154582 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/ui/keyDetailsDlg.js @@ +482,5 @@ > + { id: "key-usage-encrypt" }, > + { id: "key-usage-sign" }, > + { id: "key-usage-certify" }, > + { id: "key-usage-authentication" }, > + ]); I think it's probably better to just do this call one for each, if and when they are needed. @@ +555,1 @@ > I think subkeys must be set before this function/constructor finishes.
|
|
Reporter | |
Comment 60•4 years ago
|
||
Comment on attachment 9154583 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-2-1.patch Review of attachment 9154583 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/modules/keyUsability.jsm @@ +326,1 @@ > } You're going to use actionButtonText below, sync, so these two cases won't really work. I think it's not hooked up currently. Better to make the function async it seems ::: mail/extensions/openpgp/content/modules/keyserver.jsm @@ +59,5 @@ > + certificateError, > + importError, > + unknown, > + ] = l10n.formatValuesSync([ > + { id: "keyserver-error-aborted" }, Seems better to just localize for one, the one that will be used in the end. ::: mail/extensions/openpgp/content/modules/persistentCrypto.jsm @@ +668,1 @@ > this too won't work. mixing async with using the value sync below
|
|
Reporter | |
Comment 61•4 years ago
|
||
Comment on attachment 9154584 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-3-1.patch Review of attachment 9154584 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/modules/cryptoAPI/gnupg-keylist.jsm @@ +35,5 @@ > > +const { Localization } = ChromeUtils.import( > + "resource://gre/modules/Localization.jsm", > + {} > +); Localization is built-in nowadays so I think you should not need this. ::: mail/extensions/openpgp/content/modules/filtersWrapper.jsm @@ +22,5 @@ > */ > > const filterActionMoveDecrypt = { > id: EnigmailConstants.FILTER_MOVE_DECRYPT, > + name: l10n.formatValueSync("filter-decrypt-move2-label"), filter.decryptMove2.label and such -- this is likely a text that changed at some point, a forced l10n key change. Please just make the new fluent key something reasonable and remove the number if there's no clear use of it. ::: mail/extensions/openpgp/content/ui/enigmailKeyManager.js @@ +820,5 @@ > if (EnigmailClipboard.setClipboardContent(keyData)) { > EnigmailLog.DEBUG( > "enigmailKeyManager.js: enigmailImportFromClipbrd: set clipboard data\n" > ); > + l10n.formatValue("copy-to-clipbrd-OK").then(value => { nit: use lowercase
|
|
Assignee | |
Comment 62•4 years ago
|
||
|
|
Assignee | |
Comment 63•4 years ago
|
||
|
|
Assignee | |
Comment 64•4 years ago
|
||
|
|
Reporter | |
Comment 65•4 years ago
|
||
Comment on attachment 9154845 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-3.patch Review of attachment 9154845 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/ui/keyDetailsDlg.js @@ +87,5 @@ > document.getElementById("ownKeyCommands").removeAttribute("hidden"); > + acceptanceIntroText = "key-auto-accept-personal"; > + document.l10n.formatValue("key-type-pair-2").then(value => { > + setLabel("keyType", value); > + }); since this is an async function to begin with, better to use await @@ +92,5 @@ > } else { > document.getElementById("ownKeyCommands").setAttribute("hidden", "true"); > + document.l10n.formatValue("key-type-public").then(value => { > + setLabel("keyType", value); > + }); same
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 66•4 years ago
|
||
|
|
Assignee | |
Comment 67•4 years ago
|
||
Checkin needed for Bug-1638822_openpgp-fluent-enigmail-properties-part-1, part-2 and part-3.
|
|
||
Comment 68•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/637bcc4f8125
Bug 1638822 - convert openpgp strings to Fluent - keyDetailsDlg.js and enigmail.ftl. r=mkmelin
https://hg.mozilla.org/comm-central/rev/1cab4ddc29b6
Bug 1638822 - convert openpgp strings to Fluent - jsm files. r=mkmelin
https://hg.mozilla.org/comm-central/rev/7db4a57f1718
convert openpgp strings to Fluent - jsm files continue. r=mkmelin
|
|
||
Comment 69•4 years ago
|
||
Thanks. I've tested the patches from today on comm-beta, together with the other recent work (partially not yet uplifted to beta), and I haven't noticed any regressions yet.
|
|
||
Comment 70•4 years ago
|
||
Comment on attachment 9154847 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-2-2.patch [beta 78 checked in] needed for OpenPGP feature
|
|
||
Comment 71•4 years ago
|
||
Comment on attachment 9154848 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-3-2.patch [beta 78 checked in] needed for OpenPGP feature
|
|
||
Comment 72•4 years ago
|
||
Comment on attachment 9155935 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-4.patch [beta 78 checked in] needed for OpenPGP feature
|
|
Assignee | |
Comment 73•4 years ago
|
||
|
|
Assignee | |
Comment 74•4 years ago
|
||
|
|
Assignee | |
Comment 75•4 years ago
|
||
|
||
Comment 76•4 years ago
|
||
Comment on attachment 9154847 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-2-2.patch [beta 78 checked in] Approved for beta
|
|
||
Comment 77•4 years ago
|
||
Comment on attachment 9154848 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-3-2.patch [beta 78 checked in] Approved for beta
|
|
||
Comment 78•4 years ago
|
||
Comment on attachment 9155935 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-4.patch [beta 78 checked in] Approved for beta
|
|
||
Comment 79•4 years ago
|
||
3 more to go
|
|
||
Comment 80•4 years ago
|
||
Please let me land the beta patches, they depend on other patches, and I have them all stacked up correctly. Currently waiting for two more patches to get approved, before I can push them all.
|
|
Reporter | |
Comment 81•4 years ago
|
||
Comment on attachment 9156194 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-0.patch Review of attachment 9156194 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/ui/enigmailKeyManager.js @@ +1162,5 @@ > refreshKeys(); > }).catch(function(reason) { > EnigmailDialog.alert( > window, > + document.l10n.formatValue("general-error", { await? ::: mail/extensions/openpgp/content/ui/enigmailMessengerOverlay.js @@ +2837,5 @@ > ) { > if (callbackArg.actionType == "openAttachment") { > exitStatus = EnigmailDialog.confirmDlg( > window, > + await document.l10n.formatValue("decrypt-Ok-no-sig"), please use all lower case
|
|
Reporter | |
Comment 82•4 years ago
|
||
Comment on attachment 9156191 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-4-0.patch Review of attachment 9156191 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/modules/keyRing.jsm @@ +486,1 @@ > return ""; You need to have the out value set before you return, so this will likely not work most of the time @@ +604,5 @@ > > if (blockType.search(/^(PUBLIC|PRIVATE) KEY BLOCK$/) !== 0) { > + l10n.formatValue("not-first-block").then(value => { > + errorMsgObj.value = value; > + }); same here and the other similar cases ::: mail/extensions/openpgp/content/strings/enigmail.ftl @@ +284,5 @@ > +export-secret-key = Do you want to include the secret key in the saved OpenPGP key file? > +save-keys-ok = The keys were successfully saved > +save-keys-failed = Saving the keys failed > +specific-pub-key-filename = { $name } (0x{ $id }) pub > +specific-pub-sec-key-filename = { $name } (0x{ $id }) pub-sec doesn't seem like these should be localizable
|
|
Reporter | |
Comment 83•4 years ago
|
||
Comment on attachment 9156193 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-0.patch Review of attachment 9156193 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/modules/decryption.jsm @@ +254,5 @@ > // TODO: import key into our scratch area for new, unknown keys > if (!allowImport) { > + l10n.formatValue("key-in-message-body").then(value => { > + errorMsgObj.value = value; > + }); you need to have the out value set before returning
|
|
Assignee | |
Comment 84•4 years ago
|
||
|
|
Assignee | |
Comment 85•4 years ago
|
||
|
|
Assignee | |
Comment 86•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Comment 87•4 years ago
|
||
Comment on attachment 9155935 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-1-4.patch [beta 78 checked in] https://hg.mozilla.org/releases/comm-beta/rev/4ecb5ad3fd50d02c80d97841f55d2bbb1ba91180 https://hg.mozilla.org/releases/comm-beta/rev/decb78d3351b4bd0c43ef0b91ef01135f5778f71 https://hg.mozilla.org/releases/comm-beta/rev/02cafb5a0fe72a505f485010ec32977e80213b6d
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 88•4 years ago
|
||
Comment on attachment 9156727 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-1.patch Review of attachment 9156727 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js @@ +3356,4 @@ > ), > + label: await document.l10n.formatValue( > + "msg-compose-details-button-label" > + ), these should not really be separate, instead the button element should be using Fluent, with a label and accesskey as sub properties (or whatever it's called)
|
|
||
Comment 89•4 years ago
|
||
I found two regressions with the commited patches.
(a) openpgp key manager, click a key that never expires. The information after "expiry" is just a question mark "?". I think it was a text previously, or empty, I don't remember.
(b) openpgp key manager, delete a key. The prompt isn't properly formatted, it displays \n instead of line breaks.
|
|
Reporter | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 90•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #89)
I found two regressions with the commited patches.
(a) openpgp key manager, click a key that never expires. The information after "expiry" is just a question mark "?". I think it was a text previously, or empty, I don't remember.
(b) openpgp key manager, delete a key. The prompt isn't properly formatted, it displays \n instead of line breaks.
I will look into those and try to submit the patches by end of the day.
|
|
Assignee | |
Comment 91•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #88)
these should not really be separate, instead the button element should be
using Fluent, with a label and accesskey as sub properties (or whatever it's
called)
We are using it in appendNotification. We need to pass an array of objects which contains information related to buttons including label and accessKey. So can't do much about it.
|
|
Assignee | |
Comment 92•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #89)
I found two regressions with the commited patches.
(a) openpgp key manager, click a key that never expires. The information after "expiry" is just a question mark "?". I think it was a text previously, or empty, I don't remember.
(b) openpgp key manager, delete a key. The prompt isn't properly formatted, it displays \n instead of line breaks.
Kai, do you want to have a separate patch for this or I just apend the changes in the last part 6?
|
|
||
Comment 93•4 years ago
|
||
(In reply to Khushil Mistry [:khushil324] from comment #92)
Kai, do you want to have a separate patch for this or I just apend the changes in the last part 6?
I assume we intend to uplift all of these patches to TB 78.
If that's correct, then it's ok to include these fixes as part of your new work.
|
|
||
Comment 94•4 years ago
|
||
(To clarify, I'd like all openpgp fixes to land for 78. At this time, there is only one reason to exclude patches: If a change is necessary because of platform changes post 78.)
|
|
Assignee | |
Comment 95•4 years ago
|
||
Magnus has reviewed all the part-4, part-5, and part-6. I have updated part 6 with all the changes. Can you final check these changes?
|
|
||
Comment 96•4 years ago
|
||
Comment on attachment 9157392 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-2.patch I looked at the delta of the previous part 6 to part-6-2. You mostly changed embeeded \n characters to be real line breaks in the fluent file. And to fix, you added JS code in some places that adds line breaks when combining strings. This seems fine. You changed the identifier key-type-pair-2 by removing the -2 suffix. I assume you're certain its fine to change this string ID. I've tested that your change to keyDetailsDlg.js apparently fixes the expiry line display. However, I saw a new regression. For an expired key, the lower contents area shows "Key expired on {$keyExpiry}" without replacing the variable. I'll mark this patch as r+, it's up to you if you want to fix this in a separate patch, or a new patch revision.
|
|
Assignee | |
Comment 97•4 years ago
|
||
We had two strings with the same key "key-expired" so it was causing problems. Corrected it in this patch.
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 98•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/b5e726d4296e
convert openpgp strings to Fluent - part 4 - jsm files continue. r=mkmelin
https://hg.mozilla.org/comm-central/rev/edee35309f49
convert openpgp strings to Fluent - part 5 - jsm files continue. r=mkmelin
https://hg.mozilla.org/comm-central/rev/06b8793bef23
convert openpgp strings to Fluent - part 6 -jsm files continue. r=mkmelin
|
|
||
Comment 99•4 years ago
|
||
Comment on attachment 9156726 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-4-1.patch [c-c 79 checked in] [beta 78 checked in] needed for OpenPGP
|
|
||
Comment 100•4 years ago
|
||
Comment on attachment 9156727 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-1.patch needed for OpenPGP
|
|
||
Comment 101•4 years ago
|
||
Comment on attachment 9157558 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-3.patch needed for OpenPGP
|
|
Reporter | |
Comment 102•4 years ago
|
||
Going to have to back out part 5 and 6 for lots of test failures. https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=a55755a0f3fe6e48e615cc6c54edcb8df30fdc9e
E.g. comm/mail/test/browser/smime/browser_multipartAlternative.js and comm/mail/components/extensions/test/browser/browser_ext_composeScripts.js
|
|
Reporter | |
Comment 103•4 years ago
|
||
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Comment 104•4 years ago
|
||
key-revoked is another duplicate
|
|
||
Comment 105•4 years ago
|
||
Comment on attachment 9156726 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-4-1.patch [c-c 79 checked in] [beta 78 checked in] This patch was landed on m-c. Only affects openpgp. We need openpgp directory synched with beta 78.
|
|
||
Updated•4 years ago
|
|
|
||
Comment 106•4 years ago
|
||
Khushil, I assume you still have work in progress patches.
Could you please include the following typo fix (from bug 1647104) in your changes?
Thanks
-filter-decrypt-move-warn-experimental = Warning - the filter action "Decrypt permanently" may lead to destroyed messages.\n\nWe strongly recommend that you first try the "Create decrypted Copy" filter, test the result carefully, and only start using this filter once you are satisified with the result.
+filter-decrypt-move-warn-experimental = Warning - the filter action "Decrypt permanently" may lead to destroyed messages.\n\nWe strongly recommend that you first try the "Create decrypted Copy" filter, test the result carefully, and only start using this filter once you are satisfied with the result.
|
|
||
Comment 107•4 years ago
|
||
Comment on attachment 9156726 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-4-1.patch [c-c 79 checked in] [beta 78 checked in] Approved for beta
|
|
||
Comment 108•4 years ago
|
||
Comment on attachment 9156726 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-4-1.patch [c-c 79 checked in] [beta 78 checked in] https://hg.mozilla.org/releases/comm-beta/rev/877b593dafb5fb1f57356482c968aced8c637e79
|
|
Assignee | |
Comment 109•4 years ago
|
||
Changes:
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -56,7 +56,7 @@ var { ExtensionParent } = ChromeUtils.im
"resource://gre/modules/ExtensionParent.jsm"
);
-var l10n = new Localization(
+let l10n = new Localization(
["messenger/messengercompose/messengercompose.ftl"],
true
);
|
|
Assignee | |
Comment 110•4 years ago
|
||
Added solutions to all the regression by the previous patches.
Changes suggested by Kai:
-filter-decrypt-move-warn-experimental = Warning - the filter action "Decrypt permanently" may lead to destroyed messages.\n\nWe strongly recommend that you first try the "Create decrypted Copy" filter, test the result carefully, and only start using this filter once you are satisified with the result.
+filter-decrypt-move-warn-experimental = Warning - the filter action "Decrypt permanently" may lead to destroyed messages.\n\nWe strongly recommend that you first try the "Create decrypted Copy" filter, test the result carefully, and only start using this filter once you are satisfied with the result.
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 111•4 years ago
|
||
Comment on attachment 9159530 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-2.patch You may remove this part in encryption.jsm because the code has changed. ``` @@ -377,10 +375,9 @@ var EnigmailEncryption = { } if (keyList.length === 0) { - ret.errorMsg = EnigmailLocale.getString( - "errorOwnKeyUnusable", - fromMailAddr - ); + ret.errorMsg = l10n.formatValueSync("error-own-key-unusable", { + addr: fromMailAddr, + }); return ret; } ``` Also, you may remove string error-own-key-unusable that scenario is now reported differently.
|
|
Assignee | |
Comment 112•4 years ago
|
||
Sure, I will rebase both the patches again.
|
|
||
Comment 113•4 years ago
|
||
It's just a small amount of code. Because I know what I changed, it might be easier if I do it. I'll attach updated revisions shortly.
|
|
Assignee | |
Comment 114•4 years ago
|
||
I have done it here: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3920d0920e9a6abfd7ba8696bfe2ec9ff706ad83
If the tests are good, I will upload these patches.
|
|
||
Updated•4 years ago
|
|
|
||
Comment 115•4 years ago
|
||
Khushil, thank you. Your patches look good. Let's use your patches.
|
|
||
Comment 116•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #115)
Your patches look good.
I'm referring to the code conflict that was introduced yesterday. You have merged it well, and I've tested that this particular place works correct.
I haven't done a full review.
|
|
Assignee | |
Comment 117•4 years ago
|
||
|
|
Assignee | |
Comment 118•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 119•4 years ago
|
||
Composer is broken with the attached patch.
On opening a composer window, I get:
JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 1: SyntaxError: redeclaration of var l10n
You should completely remove the following variable declaration, which apparently is in conflict with the l10n declaration from mailWidgets.js
(Apprently the JS engine accepted your redeclaration if it was identifical to the earlier one.)
diff --git a/mail/components/compose/content/MsgComposeCommands.js b/mail/components/compose/content/MsgComposeCommands.js
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -56,7 +56,7 @@ var { ExtensionParent } = ChromeUtils.im
"resource://gre/modules/ExtensionParent.jsm"
);
-var l10n = new Localization(
+let l10n = new Localization(
["messenger/messengercompose/messengercompose.ftl"],
true
);
|
|
Assignee | |
Comment 120•4 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #119)
Composer is broken with the attached patch.
On opening a composer window, I get:
JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 1: SyntaxError: redeclaration of var l10nYou should completely remove the following variable declaration, which apparently is in conflict with the l10n declaration from mailWidgets.js
(Apprently the JS engine accepted your redeclaration if it was identifical to the earlier one.)
I am renaming the variable to l10nCompose in the MsgComposeCommands.js file so it will not create any confusion/overriding.
|
||
Comment 121•4 years ago
|
||
I am not sure how the patches here landed and modified the test behavior.
But can someone look at the warnings reported in
bug 1649019 ?
JavaScript Warning: "[fluent-dom] While translating an element with fluent ID "openpgp-search-signature-key" a child element of type "label" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type."
TIA
|
|
Reporter | |
Comment 122•4 years ago
|
||
Comment on attachment 9159721 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-3.patch AIUI from the comments, some things to fix here still. Please re-flag for review when it's done.
|
|
Reporter | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 123•4 years ago
|
||
|
|
Assignee | |
Comment 124•4 years ago
•
|
||
Included changes for bug 1649019 also so that we don't need to submit multiple beta patches.
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
||
Comment 125•4 years ago
|
||
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/97ca0281509e convert openpgp strings to Fluent - part 5 - jsm files continue. r=mkmelin https://hg.mozilla.org/comm-central/rev/abe41c820652 convert openpgp strings to Fluent - part 6 -jsm files continue. r=mkmelin
|
|
Reporter | |
Comment 126•4 years ago
|
||
Comment on attachment 9160010 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-6.patch Review of attachment 9160010 [details] [diff] [review]: ----------------------------------------------------------------- (forgot to mark r+ before landing) These went into 79.
|
|
||
Comment 128•4 years ago
|
||
We should uplift the last patches to 78.x
Sounds like the work for beta 78 is done, and trees are currently being merged.
I conclude the new uplift target is comm-esr78.
|
|
||
Comment 129•4 years ago
|
||
Comment on attachment 9160009 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-4.patch Required for future localization, required for consistency of the OpenPGP code between c-c and esr78.
|
|
||
Comment 130•4 years ago
|
||
Comment on attachment 9160010 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-6.patch Required for future localization, required for consistency of the OpenPGP code between c-c and esr78.
|
|
||
Comment 131•4 years ago
|
||
Comment on attachment 9160009 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-5-4.patch Approved for esr, given the risk and fact that pgp is preffed off by default
|
|
||
Comment 132•4 years ago
|
||
Comment on attachment 9160010 [details] [diff] [review] Bug-1638822_openpgp-fluent-enigmail-properties-part-6-6.patch Approved for esr, given the risk and fact that pgp is preffed off by default
|
||
Comment 133•4 years ago
|
||
| bugherder uplift | ||
Thunderbird 78.0.0 (build2):
https://hg.mozilla.org/releases/comm-esr78/rev/44042a27cba7
https://hg.mozilla.org/releases/comm-esr78/rev/a639ba4a6446
|
|
Reporter | |
Comment 134•4 years ago
|
||
We should remember to move the Fluent files to localization before next merge day so we get localizations going for the 78.2.
|
|
Reporter | |
Comment 135•4 years ago
|
||
Move strings to localization. (Renamed a few files at the same time, and fixed a couple of ellipsis.)
|
|
||
Comment 136•4 years ago
|
||
it seems you deleted file key-wizard.ftl
|
|
Reporter | |
Comment 137•4 years ago
|
||
Don't know what happened to that. It was supposed to be adjusted to keyWizard.ftl (matching the file it's used in)
|
|
||
Comment 138•4 years ago
|
||
Thanks. I'm briefly re-building esr78 (mostly from cache) and will doublecheck that it still works.
|
|
||
Comment 139•4 years ago
|
||
You missed this part.
r=kaie if you merge this into your patch
|
|
||
Updated•4 years ago
|
|
|
||
Comment 140•4 years ago
|
||
Well if you r+ mine we can commit both, and don't need a new patch.
But if you want to combine it, fine, too.
|
|
Reporter | |
Comment 141•4 years ago
|
||
Will fold, thx.
|
|
||
Comment 142•4 years ago
|
||
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/595dddfbd18b move OpenPGP strings out of content to l10n. r=KaiE DONTBUILD
|
|
Reporter | |
Updated•4 years ago
|
|
|
||
Comment 143•4 years ago
|
||
We need this on the esr78 branch, right?
Now or later?
|
|
||
Comment 144•4 years ago
|
||
Comment on attachment 9166118 [details] [diff] [review] bug1638822_move_openpgp_to_l10n.patch Required for Localization of OpenPGP
|
|
||
Comment 145•4 years ago
|
||
Comment on attachment 9166124 [details] [diff] [review] bond-frontend.patch Required for Localization of OpenPGP
|
|
Reporter | |
Comment 146•4 years ago
|
||
Yes needed for esr, but no need to land it for 78 just yet. It does need to be uplifted there before 78.2.
|
|
||
Comment 147•4 years ago
|
||
Comment on attachment 9166124 [details] [diff] [review] bond-frontend.patch Approved for esr78
|
|
||
Comment 148•4 years ago
|
||
Comment on attachment 9166118 [details] [diff] [review] bug1638822_move_openpgp_to_l10n.patch Approved for esr78
|
|
Reporter | |
Comment 149•4 years ago
|
||
(All done)
|
|
||
Comment 150•4 years ago
|
||
| bugherder uplift | ||
Thunderbird 78.1.1:
https://hg.mozilla.org/releases/comm-esr78/rev/4a8cf3c3de4e
Description
•