1627736 - Simpler user onboarding: Add a powerful "Personal Key Configuration Dialog" (PKCD)

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement, P1)

Description

[Kai Engert [:KaiE:]]

For various reasons, we want to require that users explicitly opt in to use OpenPGP.

However, once a user knows they want to use OpenPGP, or once they discover OpenPGP support in the Thunderbird preferences, we'd like to make it as easy as possible for them to get started.

(While in the future we might potentially have some notification bars that invite users to use OpenPG based on context, for example when reading a signed email, let's start with the obvious for now.)

We've recently added e2e encryption preferences, that allows the user to configure their own personal OpenPGP key, which is the precondition to actively use OpenPGP when sending.

With that, I see two major ways for users to discover OpenPGP support:

• users views account preferences and discovers the e2e section,
  reads that a personal key is required, and clicks the
  "set my personal key" button.
  Let's devote this bug to this scenario.

• user receives a digitally signed OpenPGP message,
  we show some status, and the user clicks it.
  If OpenPGP is not yet set up, this might be an opportunity to invite the user.
  I'll file a separate bug to track that and probably do it after this one.

Comment 1

[Kai Engert [:KaiE:]]

(In reply to Kai Engert (:KaiE:) from comment #0)

• user receives a digitally signed OpenPGP message,
  we show some status, and the user clicks it.
  If OpenPGP is not yet set up, this might be an opportunity to invite the user.

bug 1627740

Comment 2

[Magnus Melin [:mkmelin] (away, back June 30)]

Another obvious entry point would to offer it in connection to the account setup, once the account is ready to use but before you exit the setup. As kind of an additional optional step.

Comment 3

[Kai Engert [:KaiE:]]

Today, if the user clicks "set personal key" in e2e account prefs, we'll ask the user to select one from the existing personal keys related to the account's email address.

This currently assumes that the user has already used other UI to prepare a personal key. It's already possible to use the openpgp key manager dialog to either generate a new key, or to import an existing key from a file backup. This works, but obviously requires the user to be aware of it.

We should change the action of the "set personal key" button. Instead of a simple selection dialog, a more advanced dialog or wizard should be shown - maybe we can call it the "Personal Key Configuration Dialog" (PKCD).

I think the contents of the PKCD should be dynamic. It could show different contents based on what's available in the environment.

Relevant information that could influence the appearance of the PKCD:

• are there existing personal keys already?
• are there existing personal keys, but none of them is "valid" any more? (e.g. revoked or they expired, because their configured validity period has passed)
• is external GnuPG software configured, and it has a personal key available (potentially on a smartcard)?

If there are no keys yet, we'd start by showing a general introduction. A "Next" button could jump between introduction screens, and finally lead to the usual dialog that offers configuration choices.

Usual contents:

If the currently/previously selected key has expired, we could display an additional section/button, that allows the user to extend its lifetime. If the user completes the extension, we could automatically select it and close the dialog. (Optionally with a reminder/assistance to broadcast the key to contacts that used this key in the past.)

We'd always offer the button "import my personal key from a backup". If chosen, we'll try to import, requires file selection and password entry to unlock the backup. Could fail to unlock. Could fail to match, if the restored key doesn't contain the required email address for the current identity. If the import is successful and matches, we'll select it as the configured choice, and close the dialog.

Offer "use one of my existing personal keys". Shows a list or dropdown. List could be empty.

We'd always offer "create a new key". If chosen, dialog contents will change to assist and offer maybe a few choices, and then we'll automatically use this new key and close the dialog.

Optional: If we detect externally available GnuPG and personal keys available to GnuPG, we could offer to directly use it. Instead of importing, we'd remember that we have configured a dynamic connection to the externally available key. This can enable the use of smartcards managed by GnuPG which contain a personal key. If this is available, we'd show "use a personal key that's externally managed by GnuPG". Depends on implementing a GPGME interface.

Comment 4

[Kai Engert [:KaiE:]]

(In reply to Magnus Melin [:mkmelin] from comment #2)

Another obvious entry point would to offer it in connection to the account setup, once the account is ready to use but before you exit the setup. As kind of an additional optional step.

filed bug 1627762 to track this suggestion

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: generate-key.png

Let's tackle this issue by reusing as much as possible components and styles we have in the Account Settings.

The idea here is to use a SubDialog with a Generate Key button directly available there, as it follows the natural thought process of the user:

• I need to set a key, so I click on Set Personal key...
• Uh? I don't have any key in the list, and there's a Generate Key button.

The Generate Key dialog opens up and guides the user through the various steps.

We should have a check in place which warns the user if he tries to generate a new key if the current identity has already a valid key. Something like:
"Thunderbird detected a valid Key currently available for this identity, do you want to create a new Key anyway?", and maybe have a little footnote saying "Using multiple keys for the same identity is not recommended. Learn more", hyperlinked to some good Wiki article about it.

Another proposal is to implement an identity dropdown (very much identical to what we have in the Compose Window) to allow users to manage keys of multiple identities form the same location, without the necessity of closing this dialog, selecting another E2E from another identity, and clicking again to reopen the same dialog.

What do you guys think?

Comment 6

[Richard Marti (:Paenglab)]

Comment on attachment 9146625 [details]
generate-key.png

Looks similar to the S/MIME "Select Certificate" dialog, which is unfortunately not possible for me to make a subdialog.

Maybe, when where exists already a valid key, the warning text could be directly shown in the dialog to not surprise the user after clicking the "Generate Key" button with the info. But this could also add some clutter to the dialog.

Comment 7

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9146625 [details]
generate-key.png

It looks quite a bit nicer. A larger question for this, and likely other bugs in the UI effort on this is how much we want/can/have time to fix at the moment, and how much to just do incremental fixing. 

I think we need some explanation of what the listing there would be (keys that claim to belong to the specify address)

BTW, the title could better be "Set Personal OpenPGP Key for Identity" - since it would often include setup, not only selection.

Ultimately, I'm not sure it's very useful to have such a big area reserved for a keylisting there. In reasonable cases, you'd only have one key ever listed there. Maybe if you lost or changed your key, there would be 2, or maybe 3-4 at the most? 

Perhaps if there are no keys, that area should not be shown at all, and there would instead be an explanation. Like

``` 
<brandshortname> doesn't yet know about any keys for <email>. If you have an existing key, please import it.
If you don't have an existing key for this identity, you can create one now.

  [Import Existing Key... ]  [ Create New Key ]
```

Oh, and it doesn't seem there should be a cancel button (that stuff won't be cancellable). Perhaps not even an ok one.

Comment 8

[Kai Engert [:KaiE:]]

A lot of the options that I mentioned in commit 3 are missing from the initial suggestion.

Comment 9

[Kai Engert [:KaiE:]]

Because there are so many details we must present to the user, it seems that management of multiple identities adds an unnecessary complexity layer in this dialog.

We must:

• Show a list of VALID keys you have. (Agreed with Magnus, space in screenshot is too big.)
• Show a list of EXPIRED keys the user has, and offer the ability to extend the lifetime.
• Offer to import a personal/secret key from a backup file
• Offer to create a new key
• offer to configure a key from external gnupg

Here's my suggestion for the dialog.
My explanations to explain the dialog to you are {{{wrapped}}}.

To use OpenPGP, you must own a valid personal key, which consists of your 
secret key and your public key.

Please select the key that you wish to use with your identity NAME <email>.

{{{some introduction helper text, based on the situation, e.g.
   - Your current configuration uses key ID 0x1234576781287872                [cancel]
   - You previously used key ID 0x1234576781287872 which has expired
   - You currently use an external key }}}

You have the following valid personal keys.
________________________________________________
1                                                     [Use the selected key]
2                                                     
3                                                     {{{-> selects and closes the dialog}}}
external key: 4                      
________________________________________________


{{{in the above list, a single entry with external key is shown, only, 
if the user has gone through the advanced area below}}}


{{{ start of expiration section, optionally shown or hidden }}}

You have the following keys which are expired.
You may change the expiry date of an expired key 
to continue use it
________________________________________________
4
5                                                    [Change expiry date] 
                                                     {{{expiration changed in a sub dialog}}}
________________________________________________     {{{after done, we come back and update the list of valid keys}}}

{{{end of expiration section}}}


If you have previously used a personal OpenPGP key on a different    [Import personal key from backup] -> 
computer or using other software, and you want to keep using your    
key with Thunderbird, you can import a backup file of your           {{{opens file prompt, allows to import, 
personal key.                                                        then we go back and update the list above}}}
         

If you don't have a personal key yet, or if you no longer want to use     [Generate key]
one of your existing personal keys, you can generate a new personal key.


Advanced: Use a personal key that you manage with external GnuPG         [Set external personal key]
          software, possibly on a smartcard.

Comment 10

[Kai Engert [:KaiE:]]

Comment 9 is a suggestion for a powerful all-in-one dialog.

We could optionally use a wizard style approach, as described below.

start page

To use OpenPGP, you must own a valid personal key, which consists of your 
secret key and your public key.


{{{Display a combination of the following statements (dynamic as applicable)}}}


either
 - Thunderbird cannot find any valid personal key that can be used
   with email address <email> of this identity.
   [get a personal key] -> go to "key choice"
or
 - Your current configuration is valid. You are using
   key ID 0x1234576781287872 for this identity.
   [use a different personal key] -> go to "key choice"


and either
 - nothing (if key never expires)
or
 - Your personal key will expire in x months.
     [extend key lifetime] -> jump to change expiry, then come back here
or
 - The key you had previously selected has already expired
     [extend key lifetime] -> jump to change expiry, then come back here

{key choice}

Which personal key would you like to use?

 [use one of my existing personal keys] -> go to "select existing key"
 [restore a personal key from a backup] -> go to "import from file"
 [create a new personal key]            -> go to "make new key"
 
 [advanced: use a personal key that I manage with external GnuPG software]
  -> go to "enter external key id"

 [go back] -> back to start page

{select existing key}

  listbox with existing valid keys
                [use selected key]

  [go back] -> back to key choice

{import from file}

  please open the file that contains a backup of your personal key
  
  if you want to import a personal that you have previously used
  with other OpenPGP software, please refer to the other software
  for instructions how to export your secret key
  
  [open file]
  
  [go back] -> back to key choice

{make new key}

  Note that if you already have an existing personal key, it's recommended
  that you continue to use it.
  
  If you already have a personal key, please go back and import it.
  
  Please be aware of the consequences of using encrypted email.
  
  Please check that you are legally allowed to use it
  
  Please be aware that you should take care and keep a backup of your
  personal key. If you lose all copies of your personal key,
  you will no longer be able to read the copies of encrypted email
  you sent in the past
  
  [go back]

  [proceed to create new key] -> next page
  
  ---- next page
  
  key choices, key size, lifetime, with explanations
  
  [calculate random numbers for your personal key]
  
  Now you should make a backup of your personal key

{advanced, external key}

  You are responsible to set up the gnupg software on your computer
  yourself. Please type the key id of the personal key that you
  want to use.
    [edit field] 
    [test using this key]
    [ok]

  [go back]

Comment 11

[Kai Engert [:KaiE:]]

As a first step, we should decide what approach we want to implement.

Do want a all-in-one dialog as described in comment 9 ?

Or do we want a wizard-like experience with go back/forward buttons, with room for more explanations, as described in comment 10 ?

Comment 12

[Alessandro Castellani [:aleca]]

Thanks for the detailed feedback.

To quote Magnus:

A larger question for this, and likely other bugs in the UI effort on this is how much we want/can/have time to fix at the moment, and how much to just do incremental fixing.

That was my same thought, and I think aiming for small incremental improvements is the way to go.
That's why the proposed mock-up is very simple and doesn't change much other than adding the Generate Key button in it, to make ti more discoverable.

Anyway, I'll sit on these useful notes from Kai for a bit and come up with other proposals to tackle everything, and separate the implementation into small incremental fixes so are faster to review and can land progressively.

Comment 13

[Magnus Melin [:mkmelin] (away, back June 30)]

Re comment 9, I think a more wizard-like setup could be useful.

Comment 14

[Kai Engert [:KaiE:]]

Comment on attachment 9146625 [details]
generate-key.png

My recommendation is to remove the identity header. Maybe the column selector should be hidden. I'm fine to have this dialog as an intermediate step, as an initial "cleaner" dialog, but would prefer to see a more general suggestion for the options we need to offer to the user.

Comment 15

[Alessandro Castellani [:aleca]]

Attachment: dialog 1.png

All right, here's what I came up with, and apologies if it took me a bit long but I had to dive deep into all these options and discard a lot of different approaches in order to find something linear and easy to understand for the users.

First thing, let's follow the user journey.
The user wants to set up the OpenPGP, so he clicks on Set Personal Key... and this is the first dialog he will see.

• Since we don't have any keys, we hide the list to avoid confusion.
• We add a little description/explanation of what the user needs to do.
• We offer the 3 main buttons to set things up, which will open dedicated window dialogs.

We shouldn't write long or detailed descriptions like "The OpenPGP protocol requires a keya nd a fingerprint associated to your identity, etc..." as it doesn't guide the user to what he needs to do in order to set things up.

I think we can safely assume that if a user wants to set up OpenPGP, he has a general knowledge of what it is, and our job is to offer the path of least resistance to get him up to speed.

Comment 16

[Alessandro Castellani [:aleca]]

Attachment: dialog 2.png

This is the same dialog as the one I uploaded before, but it dynamically updates after the user completes one or more Key setup process, which they happen in separated dialogs.
We can manage these through callbacks, or maybe some event listener to avoid a callback hell, if the other setup dialogs were opened from within this dialog.

Nonetheless, we show an updated UI with the full list of available keys with all the info the user needs to have.

In this example, I'm showing you how the dialog react based on the status of keys.
We don't need an entire extra section to alert the user that a key has expired, but instead we can flag that entry as disabled/unselectable, and use intuitive iconography to grab the user's attention.
Clicking on that icon will prompt a dialog to cancel or update the expiration date of that key.

Comment 17

[Alessandro Castellani [:aleca]]

Attachment: dialog 3.png

Same dialog, but with a selected key.

You can see how the description area updates itself to let the user know that the system is using that highlighted key.

I think this is a good first implementation that will allow me to focus on a single dynamic sudialog, and hopefully quickly nail down this first iterative polishing process.

Comment 18

[Richard Marti (:Paenglab)]

Comment on attachment 9149581 [details]
dialog 1.png

Looks good.

As one that doesn't know much about PGP I'm a bit confused about the "Configure key from GnuPG". Maybe it would be better "Create key with GnuPG"? It's also the biggest button which can lead the user to use it as it looks most prominent. 

How about making it less prominent like a lighter button or show as link?

Maybe we could also set a focus on "Create new key" to guide the user what he normally should use?

Comment 19

[Richard Marti (:Paenglab)]

Comment on attachment 9149582 [details]
dialog 2.png

Looks good too.

For a Newby does he know what's "External"? Maybe not an issue as a Newby has no keys on a server normally.

Comment 20

[Kai Engert [:KaiE:]]

I haven't yet looked at the proposal, but want to answer Richard's questions:

(In reply to Richard Marti (:Paenglab) from comment #18)

As one that doesn't know much about PGP I'm a bit confused about the
"Configure key from GnuPG". Maybe it would be better "Create key with
GnuPG"?

No, it's specifically NOT about creating a key with GnuPG. We'll never do that. We only want to allow expert users, who have already a key in GnuPG available, to configure Thunderbird to use it. (Intended for supporting secret keys on smartcards.)

It's also the biggest button which can lead the user to use it as it
looks most prominent.

In bug 1636791 we added a pref for supporting GnuPG. If the pref is off, we could potentially hide the UI choices that are related to GnuPG, to not confuse/misled the majority of users, who we don't want to encourage to use GnuPG.

Comment 21

[Kai Engert [:KaiE:]]

(In reply to Richard Marti (:Paenglab) from comment #19)

For a Newby does he know what's "External"?

Only an expert, who has allowed the use of GnuPG (using the pref), would ever see the "external" prefix shown.
Alternatively, we could consider to show the prefix "gnupg", if that seems more obvious.

Maybe not an issue as a Newby has no keys on a server normally.

No servers involved here. External only means "outside of the area that Thunderbird directly controls, managed by other software on your computer".

Comment 22

[Richard Marti (:Paenglab)]

(In reply to Alessandro Castellani (:aleca) from comment #16)

Created attachment 9149582 [details]
dialog 2.png

This is the same dialog as the one I uploaded before, but it dynamically updates after the user completes one or more Key setup process, which they happen in separated dialogs.
We can manage these through callbacks, or maybe some event listener to avoid a callback hell, if the other setup dialogs were opened from within this dialog.

With subdialogs.js this should be easy doable like here:

  parent.gSubDialog.open(
    "chrome://messenger/content/am-identities-list.xhtml",
    null,
    args,
    onCloseIdentities
  );

The onCloseIdentities is the function that is executed when the called dialog closes. Then you can check if something changed.

Comment 23

[Richard Marti (:Paenglab)]

(In reply to Kai Engert (:KaiE:) from comment #20)

In bug 1636791 we added a pref for supporting GnuPG. If the pref is off, we could potentially hide the UI choices that are related to GnuPG, to not confuse/misled the majority of users, who we don't want to encourage to use GnuPG.

That would be good to not confuse users which have no GnuPG.

Comment 24

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9149581 [details]
dialog 1.png

I don't think we can assume the user has any background knowledge if he wants to encrypt. We should design it in such a way that if your grandmother wants to encrypt, she can do so. So, I think some explanation is necessary - but we should try to keep it short. I think it's especially important to explain that they shouldn't create a new new if they have one already. (I'm just covering one part of the problematics, but that should get the user alert.)

While texts on buttons can be helpful, it requires the user to read the text and from that decide what to push - which is not ideal.
 
I'd suggest:

```
+------------------------------------------------------------------------------------------------------------------------+
|   Thunderbird doesn't have a personal OpenPGP key for <email> yet.
|
|   If you have an existing key you've already used for this email address, you should import that. Otherwise
|   you will not have access to your archives of encrypted emails. Going forwards, you would also be likely to  
|   get incoming encrypted emails you're not able to read.
| 
|   If you do not have an OpenPGP key from the past, you can set one up now. 
| 
|   ( o ) I don't have an existing OpenPGP key for <email>
|   (   ) I have an existing OpenPGP key that I'd like to import
|
|                                                                                     [  Cancel  ]  [ Continue ]
+------------------------------------------------------------------------------------------------------------------------+
```

Comment 25

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9149582 [details]
dialog 2.png

I would suggest to drop the tree view and just use a normal list with radio buttons for which to use. 
That a selection in a tree would perform a real "selection" is unexpected.

These found keys which to choose from could just be listed in my previous selection.

Comment 26

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9149583 [details]
dialog 3.png

I think it *looks* fairly nice. But would still prefer not to use the tree for this.

Comment 27

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #15)

I think we can safely assume that if a user wants to set up OpenPGP, he has a general knowledge of what it is

I'm afraid, we cannot assume that. I think we must assume that users will discover this configuration section, they will believe that encryption is cool and they might simply try to use it, without fully understanding what they're doing.

I think we should give a little explanation. I would keep it short. But at least tell the user what this is about.

(More comments from me will follow. But I prefer to give my feedback in separate comments.)

Comment 28

[Kai Engert [:KaiE:]]

Alessandro, I like that you're using a "situational" layout of the dialog.

It seems OK to offer the same buttons/choices in the dialog, regardless of the existing configuration.

In dialog 1, if the user doesn't have any key yet, we're saving a lot of space for the omitted list.

I agree with Magnus that in this situation, we should explain a little more. But it seems, Alessandro, that you don't like the "wizard" style approach that we have suggested before (because you didn't use that idea in your mockup). That's OK with me. From my point of view, it should be OK to keep the general layout of the dialog, for all situations.

Your current text is:

"Thunderbird couldn't find any key...
Create a new key or import existing"

I think it is important that we mention the import first, to inspire users to ponder about that option. So with the inspiration from Magnus' comment 24, we could use a text like this (for the "no key yet" situation):

"Thunderbird couldn't find any PERSONAL key currently stored for ...@....
If you have previously used OpenPGP and have a backup of your personal key, please import it. Importing your existing personal key(s) will allow you to read your archived encrypted emails and future emails that your correspondents encrypt using your existing key.
If you don't have a backup of your personal key, you can create a new one."

Comment 29

[Kai Engert [:KaiE:]]

Regarding expiration. Alessandro, your suggestion to avoid a separate list for expired keys, and to use a warning symbol, is fine. However, I think your proposal needs improvement.

For accessibility and discoverability reasons, I think we shouldn't require that a user clicks the orange triangle to extend the lifetime of a key, but rather, there should be a different way to trigger that action.

Also note that it's absolutely necessary that we offer the ability to extend the key expiration IN ADVANCE. (Because if you wait until after it expires, you already have made it impossible for others to send you encrypted email. It's a best practice to renew an expiring key well in advance. This gives your communication partners a chance to receive your renewed key from you ahead of its previous expiration.)

In addition, I think it would be helpful, if this dialog allows the user to open a "key details" dialog. This might be necessary to help the user understand the difference between the shown keys. However, potentially these two actions could be behind a single button.

In other words, I think this dialog needs one or two additional buttons.

Either we add two buttons "view details" and "change key expiration".

Or, we add a single button "view and edit key". This could open our existing "key details" dialog. Inside that dialog, we could add the ability to change the expiration date of the key. (For example, we could have a button behind the expiry date that says "change expiration date").

(more comments from me later)

Comment 30

[Kai Engert [:KaiE:]]

I think we'll need a mix of dialog 2 and dialog 3 for the following situation:

• user has previously selected key A
• in the meantime key A has expired
  (and the user might get errors while using the key, so that motivated the user to come back here to the configuration, and see what needs to be done. In addition, the user might have an additional key, which has not yet expired)
• the list shows at least two keys, and the previously configured key has expired.

In this situation, the dialog should tell the user something like "Your currently selected key has expired. You may edit the key and extend its lifetime. Alternatively, you can select a different valid key, or create a new key."

Comment 31

[Kai Engert [:KaiE:]]

You have added an extra column on the left, "User ID".

We only allow keys that include the current email address (the one we're configuring). And if the user has multiple keys, the user might have set the same own name for all their own keys.

So, it's likely that we'd show the exact same text for all keys in that column.

In order to fully understand which key is what, it's probably necessary to have a mechanism to open the full details of the listed keys for inspection. This is another reason to add the "key details" button that I've suggested in comment 29.

Comment 32

[Alessandro Castellani [:aleca]]

Thank you all for the detailed explanation and feedback.
I think we're getting closer, and my failed attempts are helping us to highlight all the major cases and possible problematics.
I'll get back to the drawing board and update the mock-ups.
Cheers

Comment 33

[Alessandro Castellani [:aleca]]

Attachment: first.png

Let's focus on the first screen which is a bit of a standalone dialog, as the user will likely not see this again after the first setup.

I tried to stick to the Wizard-like approach, since based on the steps and options you highlighted, it seems to be the most flexible solution to onboard first time OpenPGP users.

I have 2 approaches here.
The first is more common and requires the user to select an option from the radio list before continuing.
The second mimics the Account Central, which might be more familiar to help the user understand that this is a multi-step setup journey.
In the second example, the first button is "hovered" which causes a tooltip description to appear, exactly like it happens in the Account Central.

In both cases, I added the important description regarding existing keys inside an inf-like notification box.
I think we should separate it from a regular description as it's very important and prevents mistakes.

I reworded the text a bit, and added a Learn more link which it could point to a Wiki page with a more detailed overview of what might happen when not using an existing key.

Comment 34

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9150347 [details]
first.png

Looks good. I like the top one at least. 

In the info text, you could add " ... from people who are still using your existing old key."

The GnuPG option we'd only show after setting a pref, right? I forget the exact details. If it is what I think, I guess it should be "Use external key through GnuPG (e.g. from a OpenPGP card)"

Comment 35

[Alessandro Castellani [:aleca]]

(In reply to Magnus Melin [:mkmelin] from comment #34)

Looks good. I like the top one at least.

Awesome. Yes, the top one is more common and intuitive.

In the info text, you could add " ... from people who are still using your existing old key."

Sure.

The GnuPG option we'd only show after setting a pref, right?

Correct.

I forget the exact details. If it is what I think, I guess it should be "Use external key through GnuPG (e.g. from a OpenPGP card)"

This was Kai's explanation:

If we detect externally available GnuPG and personal keys available to GnuPG, we could offer to directly use it. Instead of importing, we'd remember that we have configured a dynamic connection to the externally available key. This can enable the use of smartcards managed by GnuPG which contain a personal key. If this is available, we'd show "use a personal key that's externally managed by GnuPG". Depends on implementing a GPGME interface.

So yeah, I think something like "Use your external key through GnuPG (e.g. from a smartcard)" would be good.

I'll update the mock-up and move onto the second screen.

Comment 36

[Alessandro Castellani [:aleca]]

Attachment: first.png

Mock-up updated with Magnus' feedback.

Comment 37

[Alessandro Castellani [:aleca]]

Attachment: second.png

Here's the second step of the same dialog, after a user has created 1 or more keys.
This UI uses the same paradigm in Firefox Privacy preferences settings.
Radio buttons are a more intuitive "selector" and we can use the expandable container to show extra info if the user needs.

Comment 38

[Alessandro Castellani [:aleca]]

Attachment: second-selected.png

This example, shows what a user might see once a valid key has been selected.

• The title updates to "Manage" instead of "Set".
• The key icon is coloured with our primary colour, and a green check accompanies the message highlighting which key is currently being used.
• The selected key is properly highlighted.
• A generic "Add new key" button is always present, which will redirect the user to the first mockup to once again pick the action to add a new key.

Comment 39

[Alessandro Castellani [:aleca]]

I was thinking that we might even consider getting rid of these dialogs and have all of these views inside the settings page itself, exactly like the Privacy preferences from Firefox.
We might use dialogs only to create and edit keys, but the actual overview of currently configured keys, and the selection of the current key might live directly in the Account Settings page without the necessity of being in a separated dialog.

Comment 40

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9150637 [details]
second.png

Looks good. 
For the buttons at the bottom, maybe there should just be an Ok (Or close) instead of Cancel and Select. 
Especially Select is a bit odd for wording this, and canceling is a bit tricky. It wouldn't (easily) cancel/remove e.g. a new key you created.

Comment 41

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9150638 [details]
second-selected.png

Would put the external tag somewhere else so that the ids line up. And not sure if there's any need to have an Update showing in that overview? It could be available once you click it open. At least if you already have keys to choose from, the one that expired should probably just be left expired and only be used to access old encrypted mails. 

For the text, I think it's associated *with*, and identity is probably wrong word here. It's email. Say you have several identities (e.g. one with signature and one not), but the same email, we'd want to show all for the same email in this dialog.

So perhaps "...  associated with the e-mail address <email>." Or perhaps even shorter, ".... keys for the e-mail address <email>."

Comment 42

[Alessandro Castellani [:aleca]]

Attachment: second-selected.png

Thanks for the feedback.
Here's a step further.

Is it worth offering the user a "None" selection to allow stop using the current key?
Having this selection ability will allow us to remove a Confirm/OK and Close button as the selection automatically applies the setting, so a simple Close to close the dialog is sufficient.

Also, with this type of selection option, we could definitely move this entire dialog inside the e2e tab directly.

Comment 43

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9150786 [details]
second-selected.png

If you select none I don't think it will make your emails non-readable. You would still *have* the key which is what matters. Until you'd delete it of course. If you select none it just means "Do not use OpenPGP for this identity".

Would lose the colon in the first sentence.

Nit: for the screenshot, the ids should start with 0x

"Learn more" is a bit oddly placed. Might not be needed in this dialog.

Capitalize "Add New Key... "

Comment 44

[Alessandro Castellani [:aleca]]

Attachment: second-selected.png

Comment 45

[Alessandro Castellani [:aleca]]

Attachment: Tab.png

This is how it might look directly in the Tab, mimicking the Privacy section of FF.
Not using a dialog removes a lot of spacing issues, and the expectation of "cancellability" of a selection.

Comment 46

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9150836 [details]
Tab.png

Yep!

Comment 47

[Alessandro Castellani [:aleca]]

Kai, what do you think about this progress?
What about the copy?
Also, can you give me a screenshot of all the info we want to list when opening the Key details area?

If you think this is at a good state, I can start coding it.

Comment 48

[Kai Engert [:KaiE:]]

Comment on attachment 9150635 [details]
first.png

Looks good, I have some suggestions.

Optional: On opening, would it make sense to have all options unselected? Or would that be weird? Idea is to prevent users from clicking continue without making a choice?

Note: Users might have used several keys over time. If they do, they might have stopped using some of them, and only use one current key for active use. So for proceeding with the configuration here, importing the most recent one, the one that they still want to use, is sufficient. However, for accessing their their complete archive, they'd have to import several keys. Also, I've been told in the past it's better to use a positive wording, and avoid negated wordings.

Suggested improvement for the blue box:
"If you have an existing personal key for this email address, then you should import it. Importing your old keys will allow you to read your archives of encrypted emails, and new emails from other people who still encrypt using your old keys."

As said, if mail.openpgp.allow_external_gnupg is false, then hide the GnuPG option.

Comment 49

[Kai Engert [:KaiE:]]

Extra explanations regarding external gnupg.

The current UI design assumes that we're always able to query the availability of of all keys and their properties, including those for the external GnuPG.

Unfortunately, a smartcard can be "unplugged". If it's not inserted currently, we might not even know that it exists, and we likely aren't able to query its properties.

If the user opens the dialog while the smartcard is missing, our dynamic logic for this dialog would fail. We couldn't offer the smartcard key in the list of choices. So, for a user who previously has configured a smartcard, who temporarily unplugged it, and who opens and closes this dialog, we might corrupt the user's existing configuration.

I think the advanced configuration could be without any convenience and details at all.

It might be totally fine to have an edit field for the external key, where the user is required to paste the correct key ID that the user obtains elsewhere. We wouldn't show any details of that key. And if the smartcard is currently unplugged, we wouldn't care, we wouldn't detect it, we wouldn't tell the user. Opening the dialog would show the value the user has entered. Unless the user changes it, that information remains unchanged, even if the user closes/confirms this dialog, even if the smartcard is absent. (Thunderbird would only care that the smartcard is available when actively required, outside the preferences context, where we'd show errors like "cannot access configured external key")

Therefore I'd suggest to adjust this dialog: Treat the "external" key special, with a selector that contains an edit field and the word "external GnuPG key" and no other details.

Comment 50

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #38)

• A generic "Add new key" button is always present, which will redirect the user to the first mockup to once again pick the action to add a new key.

I like that you'll always get back to the first step, which offers both import and create.

However, it seems that "import" isn't about a new key, it's rather an old key. If I was looking for a way to import my existing key, might be confusing to have to click a button that says its about a "new key".

Based on that, should we rename the button to "Add a key" ?

Comment 51

[Kai Engert [:KaiE:]]

To answer Magnus' question:

(In reply to Magnus Melin [:mkmelin] from comment #40)

For the buttons at the bottom, maybe there should just be an Ok (Or close)
instead of Cancel and Select.
Especially Select is a bit odd for wording this, and canceling is a bit
tricky. It wouldn't (easily) cancel/remove e.g. a new key you created.

This dialog doesn't create a key, it's only about selection.

At the time you open the dialog, there can be an existing choice, your "currently" configured key.

If the user selects a different key in this dialog, and then clicks cancel, your experimental clicks are discarded, and you stay with the previous configuration.

Select confirms your new choice to be permanent.

Comment 52

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #39)

I was thinking that we might even consider getting rid of these dialogs and have all of these views inside the settings page itself, exactly like the Privacy preferences from Firefox.
We might use dialogs only to create and edit keys, but the actual overview of currently configured keys, and the selection of the current key might live directly in the Account Settings page without the necessity of being in a separated dialog.

My personal opinion is, I find this kind of immediate action risky. It makes it difficult for a user to "click to explore, and cancel if I'm unsure I did things correctly".

Especially with a delicate setting as your personal key, which will affect the actions of other people (you will advertise this new key, other people use it and encrypt to you), I would prefer to avoid a situation where a single click on this screen is sufficient to change your configuration.

Also, if you have accidentally clicked on a key - you no longer know which key was previously selected! Having a cancel button is a great relief IMHO. Especially, if someone else has helped you to setup your configuration, and you're simply exploring which settings this software has to offer. Think of people who are challenged and sometimes accidentally click somewhere?

Comment 53

[Kai Engert [:KaiE:]]

(In reply to Magnus Melin [:mkmelin] from comment #41)

And not
sure if there's any need to have an Update (expiry) showing in that overview? It
could be available once you click it open. At least if you already have keys
to choose from, the one that expired should probably just be left expired
and only be used to access old encrypted mails.

No.

You might have missed the opportunity to extend the expiration of your key in advance. But it might still be best to continue using it.

I think we need this:

• always show expiration date (except for external key - advanced users must know what they are doing)
• if the key will expire within the next 6 months, then show an orange warning triangle in front of the "expires on", keep the normal grey text, and show the update button
• if the key has already expired, then use your suggestion, orange triangle, red text, and show the update button.

You can omit the update button if the key expires later than in 6 months, I think.

Regarding the highlighting, I think it is important to warn about both "expire soon, within 6 months" and about "already expired". The "already expired" should be a stronger visual warning.

Comment 54

[Kai Engert [:KaiE:]]

(In reply to Magnus Melin [:mkmelin] from comment #41)

For the text, I think it's associated with, and identity is probably wrong
word here. It's email. Say you have several identities (e.g. one with
signature and one not), but the same email, we'd want to show all for the
same email in this dialog.

So perhaps "... associated with the e-mail address <email>." Or perhaps
even shorter, ".... keys for the e-mail address <email>."

I agree with this suggestion, and I'd prefer to add the word "personal". I'd say:

"Thunderbird found x personal OpenPGP keys for your email address [e-mail]"

Note that the statement "Thunderbird found" would be incorrect if the user has manually configured an external key. If the smartcard is absent, then it's probably wrong to even claim that we have "found" it. (And in theory, that external key might not even include an address.)

If you want to avoid this ambiguity, you could exclude the "number" of keys that were found and say:
"Personal keys for your email address [e-mail]"

Comment 55

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #42)

Is it worth offering the user a "None" selection to allow stop using the current key?

Yes, absolutely, as available with the "clear" button in the existing UI.

We must allow the user to stop using OpenPGP.

Comment 56

[Kai Engert [:KaiE:]]

(In reply to Magnus Melin [:mkmelin] from comment #43)

If you select none I don't think it will make your emails non-readable. You
would still have the key which is what matters. Until you'd delete it of
course. If you select none it just means "Do not use OpenPGP for this
identity".

Correct.

If you revert to "no key configured for OpenPGP", then you can no longer sign messages, and when sending out emails, you can no longer "attach your own public key" with a simple action, because we don't know which key that would be.

Furthermore, we don't allow you to send encrypted emails out, because the choice "don't have a personal key configured" is treated as "I don't want to use email encryption".

Comment 57

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #47)

Kai, what do you think about this progress?

It's very good progress in general, but I hope my suggestions make sense.

What about the copy?

I don't follow, which copy?

Also, can you give me a screenshot of all the info we want to list when opening the Key details area?

The full details are currently implemented in mail/extensions/openpgp/content/ui/keyDetailsDlg.xhtml which is a dynamic dialog with tabs. It would be easiest to simply have a [details] button and open that dialog. If you feel we should avoid that, then we'll have to select a "reasonable subset" of the information, which is at least:

• the list of alternative "Name (comment) <email>" user IDs (zero, or multiple) that are embedded in the key (Thunderbird currently doesn't create such complex keys with multiple IDs, but imported keys might have it, and showing this information might help the user to distinguish their keys.)
• key creation date
• full fingerprint ("0x" + 40 hex characters)
• type of cryptographic algorithm, e.g. RSA or ECC. (This will be imprecise if its a complex key that uses various mechanisms - this detail can only be displayed correctly by opening the details dialog, and showing the tree of its inner structure. It might be ok to be imprecise, because Thunderbird itself will only create keys with a simple structure.)

Comment 58

[Magnus Melin [:mkmelin] (away, back June 30)]

(In reply to Kai Engert (:KaiE:) from comment #48)

Also, I've been told in the past it's better to use a positive wording, and
avoid negated wordings.

In general yes, but I think it's worth pointing out the negative impact here instead of the positive: If you do not import an existing key you will be in trouble for existing mails using that key.

Comment 59

[Alessandro Castellani [:aleca]]

Apologies for the lack of updates on this.
I'm working on it but there are a lot of moving pieces intertwined together which don't allow me to split the update in smaller patches. This is gonna bit a bit big.

How do we want to handle new strings? Should I use/create a fluent file for them?

Comment 60

[Magnus Melin [:mkmelin] (away, back June 30)]

Strings are being moved to Fluent wholesale in bug 1638822. Lets get that landed first (hoping later today), or base your patch on it.

Comment 61

[Khushil Mistry [:khushil324]]

(In reply to Alessandro Castellani (:aleca) from comment #59)

Apologies for the lack of updates on this.
I'm working on it but there are a lot of moving pieces intertwined together which don't allow me to split the update in smaller patches. This is gonna bit a bit big.

How do we want to handle new strings? Should I use/create a fluent file for them?

Are you going to add static strings(DTD like) or through Stringbundle(properties like string)?

Comment 62

[Alessandro Castellani [:aleca]]

(In reply to Khushil Mistry [:khushil324] from comment #61)

Are you going to add static strings(DTD like) or through Stringbundle(properties like string)?

Both, some strings are static, some have variables.
I can wait an extra day for bug 1638822 to land, otherwise I'll rebase my patch on top of your WIP patches.

Comment 63

[Khushil Mistry [:khushil324]]

I will convert the static-like strings by EOD. The patch is almost ready, just a few changes. I am doing them right now.
For variable strings, you can add them in fluent file enigmail.ftl. There are around 300-400 odds strings in enigmail.properties file so it will take a few days. I will rebase/combine all those strings on your patch.

Comment 64

[Alessandro Castellani [:aleca]]

(In reply to Khushil Mistry [:khushil324] from comment #63)

I will convert the static-like strings by EOD. The patch is almost ready, just a few changes. I am doing them right now.
For variable strings, you can add them in fluent file enigmail.ftl. There are around 300-400 odds strings in enigmail.properties file so it will take a few days. I will rebase/combine all those strings on your patch.

Perfect, thanks for the heads up!

Comment 65

[Alessandro Castellani [:aleca]]

Attachment: openpgp-wizard.gif

A little preview of the WIP wizard.

Comment 66

[Alessandro Castellani [:aleca]]

I'm getting really close with this patch, sorry for the wait, it's very complex.

I have a couple of question regarding the workflow of generating a new key.

1. I noticed that the progress bar is not really used as the call to generate the key is made synchronously and then the bar gets filled at 100% at the end, without a real async progress. Can we remove the bar or is there a way to use the async method and have a real progress feedback from the genKey() method?
2. Sometimes, we're throwing exceptions during the keygen process but we're not updating the UI to reflect this errors. Is this intentional? I was already working to implement an inline error messaging to handle this situation, but I wanted to doubje check.
3. I noticed we're using a lot popup dialogs for simple warnings, like if the expiration date is too long or too short. I'd like to remove those and validate those fields on typing, like we do in the emailWizard dialog. Until all those scenarios are valid, the Generate Key button would be disabled.
4. Upon creation of the revocation certificate, we're using non localized strings hard coded in the JS file. Is this correct? Shouldn't those strings be translatable as well?

Comment 67

[Magnus Melin [:mkmelin] (away, back June 30)]

(In reply to Alessandro Castellani (:aleca) from comment #66)

1. I noticed that the progress bar is not really used as the call to generate the key is made synchronously and then the bar gets filled at 100% at the end, without a real async progress. Can we remove the bar or is there a way to use the async method and have a real progress feedback from the genKey() method?

Bug 1617444 - could be done but not yet implemented. I'd use an indeterminate progress bar (html:progress, no value) for indicating. Even with async we wouldn't get any real progress.

For me the generation has always been super fast. I think we probably don't need the warning that it can take minutes (when a second or so is normal, YMMV).

2. Sometimes, we're throwing exceptions during the keygen process but we're not updating the UI to reflect this errors. Is this intentional? I was already working to implement an inline error messaging to handle this situation, but I wanted to double check.

I'm assuming not intentional. What errors do you see?

4. Upon creation of the revocation certificate, we're using non localized strings hard coded in the JS file. Is this correct? Shouldn't those strings be translatable as well?

IIRC intentional - since the revocation certificate would be published to others who may not know your language. (You could add a code comment about this.)

Comment 68

[Kai Engert [:KaiE:]]

Alessandro, we had not yet discussed how the details of the "generate key" or "import key" would work. You could work on everything else, and keep those two dialogs unmodified for now.

I will have things to say about those actions. We might want to track them in a separate bug.

Comment 69

[Kai Engert [:KaiE:]]

For example: When the user generates a new key, we should use that opportunity for a bit of education. Mention the consequences (should backup your key, encrypted items in sent folder).

Comment 70

[Alessandro Castellani [:aleca]]

I'm assuming not intentional. What errors do you see?

Throughout the creation process we're throwing these errors:

• GetEnigmailSvc failed
• key generation failed
• failed to obtain revocation for key " + gGeneratedKey

In all of those cases the UI doesn't update and there's no feedback for a user not looking at the terminal.

Alessandro, we had not yet discussed how the details of the "generate key" or "import key" would work. You could work on everything else, and keep those two dialogs unmodified for now.

Unfortunately that's really tricky for me to split these as the whole onboarding workflow is tied together.
To avoid issues, I'm not touching those dialogs but rather implement new ones with the same methods but updated UI in order to slowly transition towards those.

I'll have a patch ready to be tested soon. We can then consider if it's better to split the patch in different bugs, but I'd rather tackle the entire user onboarding process at once.

Comment 71

[Alessandro Castellani [:aleca]]

For example: When the user generates a new key, we should use that opportunity for a bit of education. Mention the consequences (should backup your key, encrypted items in sent folder).

Definitely we should do that, but I don't think we should do it in the dialog as we risk to not have enough space and with the dismissable nature of a dialog we risk to not grab the attention of the user at the end of the generation process.
I think we should have these info in the e2e page, always visible with an introductory test and then redirecting to a support page for a more in-depth overview.

I was planning to also implement some warnings/confirm when the user switches key (if he has more than one) or when he decides to stop using a key.

Comment 72

[Alessandro Castellani [:aleca]]

Attachment: 1627736-openpgp-setup.diff

All right, finally here's something you can test.

In this WIP patch I implement the new UI for the creation of a new key with a wizard and the selection of a key in the e2e section.

This is still a WIP, lots of things are broken/not working properly/kinda working.
I still need to implement the info of the keys, expanding and collapsing of those info, deletion of the keys, warning flag when a key is near expiration, and so on.

Please focus on the following workflow:

• Start with no keys if you can and access the e2e encryption section
• If you already have keys and using one, the auto selection should work.
• Click on Add Key button, and play around with the wizard, select the various options, go back and forth, and so on. (Only the creation section is complete, the other sections have placeholders for now).
• Create a new key and see the UI updating.

I created a separated dialog for the wizard in order to not touch the current ones and prevent full breakage. Once the new UI is completed, I will remove all the duplicated files.

Comment 73

[Richard Marti (:Paenglab)]

Comment on attachment 9156184 [details] [diff] [review]
1627736-openpgp-setup.diff

This looks very good. I have only small things:
- when I proceed to generating the key, a new dialog appears to confirm the generation. Couldn't this be done inside of the existing dialog?
- Also at the generating stage the dialog content is smaller. Have you tried to resize the dialog with `resizeDialog()`?
- When going back to the start, the dialog size is still big.

Import a key doesn't work yet? I see only an almost empty dialog.

Comment 74

[Richard Marti (:Paenglab)]

Attachment: success-notification-not-centred.png

The success notification content isn't vertically centred. You could use align-items: center; instead of flex-start. And instead of using a new class notification-close with own styles, you could simply use .close-icon. This would also fix the colour on dark system theme.

Comment 75

[Alessandro Castellani [:aleca]]

(In reply to Richard Marti (:Paenglab) from comment #73)

• when I proceed to generating the key, a new dialog appears to confirm the generation. Couldn't this be done inside of the existing dialog?

Yup, that's something I still have to fully integrate. The confirm dialog is the old method.

Also at the generating stage the dialog content is smaller. Have you tried to resize the dialog with resizeDialog()?
When going back to the start, the dialog size is still big.

I tried and it doesn't work. That's a limitation of the SubDialog object, which only resizes itself to grow and not if the content is smaller than the current size.
We also use this logic (only resize when the content exceeds the available area) to prevent size jumping between big and small at every step as that might be jarring.

Import a key doesn't work yet? I see only an almost empty dialog.

Not implemented yet :D

The success notification content isn't vertically centred. You could use align-items: center; instead of flex-start. And instead of using a new class notification-close with own styles, you could simply use .close-icon. This would also fix the colour on dark system theme.

Thanks for the feedback, I'll update it accordingly.

Comment 76

[Alessandro Castellani [:aleca]]

Attachment: 1627736-openpgp-setup.diff

A little step forward and some improvements.

• Confirmation before generating a key is now inline (the UI still freezes after clicking confirm, but that's expected and not related to this bug).
• A green check appears when a valid key is selected.
• Switching between keys now properly updates pref and used key.
• Added the arrowhead toggle icon (not working yet).

More to come...

Comment 77

[Richard Marti (:Paenglab)]

Cosmetical issue: When I switch in the End-to-End Encryption pane between the "None" key and my generated key, the whole content below jumps. This comes from the empty content-blocking-extra-information which has a 18px margin-bottom. Is this margin needed or could this <vbox> be only shown, when where is some content in it?

Comment 78

[Richard Marti (:Paenglab)]

Comment on attachment 9156448 [details] [diff] [review]
1627736-openpgp-setup.diff

Looks good. I have an expired key and it isn't shown in the pane but in the manage key dialog. Will this change?

Comment 79

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #71)

Definitely we should do that, but I don't think we should do it in the dialog as we risk to not have enough space and with the dismissable nature of a dialog we risk to not grab the attention of the user at the end of the generation process.

Possible idea: The wizard can be multiple steps. Once the user clicks "create key", then the next page could be explanations only, without any action controls. Only "next".

Will try your patch.

Comment 80

[Kai Engert [:KaiE:]]

e2ee prefs pane:

nit: on linux, there is zero space between the "manage openpgp keys" button and the S/MIME section heading

nit: the "learn more" link is seen twice, after the intro message "either openpgp or s/mime", and also after the "your current configuration uses ...."

There's a bug in the preference naming.
Before the patch, I had:
user_pref("mail.identity.id1.openpgp_key_id", "847245F87CFF9221");

When starting with your code for the first time, that key is listed as one of the options, but it isn't selected. Selected is "do not use openpgp".
If I click 847245F87CFF9221 to fix it, then quite, and look at prefs.js, I see:
user_pref("mail.identity.%identitykey%.openpgp_key_id", "847245F87CFF9221");
user_pref("mail.identity.id1.openpgp_key_id", "847245F87CFF9221");

Please fix the code to not use the "mail.identity.%identitykey%.openpgp_key_id", but rather the real pref name, which should fix the "key isn't selected bug".

Comment 81

[Alessandro Castellani [:aleca]]

(In reply to Richard Marti (:Paenglab) from comment #77)

Cosmetical issue: When I switch in the End-to-End Encryption pane between the "None" key and my generated key, the whole content below jumps. This comes from the empty content-blocking-extra-information which has a 18px margin-bottom. Is this margin needed or could this <vbox> be only shown, when where is some content in it?

That's still a WIP, but it's normal as that section will be filled with the Key info and automatically showed when the user selects the key.
It's the same paradigm used in FF when you change the privacy level and the full overview of the option you selected is revealed.

nit: on linux, there is zero space between the "manage openpgp keys" button and the S/MIME section heading

I see a proper empty space with consistent separation. Could you share a screenshot? Richard, can you confirm the issue?

nit: the "learn more" link is seen twice, after the intro message "either openpgp or s/mime", and also after the "your current configuration uses ...."

Those are placeholders for now, also because the links point to a 404. I think we should have both with the first pointing to the generic article and the second pointing to a subsection or a detailed explanations of the pros/cons of using an OpenPGP Key.

Please fix the code to not use the "mail.identity.%identitykey%.openpgp_key_id", but rather the real pref name

Ouch, sorry about that, I was planning to replace those but I forgot to do it. Thanks for noticing.

Comment 82

[Richard Marti (:Paenglab)]

(In reply to Alessandro Castellani (:aleca) from comment #81)

nit: on linux, there is zero space between the "manage openpgp keys" button and the S/MIME section heading

I see a proper empty space with consistent separation. Could you share a screenshot? Richard, can you confirm the issue?

This comes from your removal of the <html:div> around the <html:fieldset>. Please put them back and the issue is gone. Without the div, the fieldset doesn't work correctly when additional line wraps are needed.

Comment 83

[Alessandro Castellani [:aleca]]

Comment on attachment 9156448 [details] [diff] [review]
1627736-openpgp-setup.diff

Removing the feedback request on this as I've improved a lot the code and almost completed the whole implementation.
A full review request is coming soon.

Comment 84

[Alessandro Castellani [:aleca]]

Attachment: 1627736-openpgp-setup.diff

Here's the patch ready for a review.
Some things are missing and others are not super polished, here's a list of what you can expect:

Things currently NOT WORKING

• Import Key/GnuPG Key not yet implemented, I will do those in a follow up bug
• Edit and Revoke key not yet implemented.
• The Key Wizard UI freezes when generating a key, this is a known problem not related to this bug.

Things that NEED POLISH in a follow up

• The Key Properties SubDialog will be dropped to list those tabs inside the collapsible area.
• The Manage OpenPGP Keys dialog will be turned into a SubDialog and get a new UI.

I didn't implement these items above because this patch is already pretty big as it is and it's getting hard to track and review.

This will most likely bitrot once bug 1638822 lands.

Comment 85

[Kai Engert [:KaiE:]]

I haven't yet looked at the code, but started to play with the behavior.
I found a bug:

• have two email accounts
• open account prefs
• click e2ee for account 1
• click e2ee for account 2
• again click e2ee for account 1

The view isn't updated, console shows:
JavaScript error: chrome://global/content/preferencesBindings.js, line 46: Error: preference with id 'mail.identity.id1.openpgp_key_id' already added

Comment 86

[Kai Engert [:KaiE:]]

If you have a key already for an account, however, the if the account's openpgp_key_id preference is absent, then the list is shown, but all shown options are unselected. The "none" entry should be selected in this scenario.

(The "none" entry is correctly selected, if the pref is present and is set to the emtpy string. So I assume the current code doesn't handle the "no pref found" scenario.)

Comment 87

[Kai Engert [:KaiE:]]

Currently, with this patch, after creation of the new key is completed, the user is left with their previous configuration. (None or the previously selected key.) Is this intentional?

Should we add another step to the wizard? Like: "Key generation completed. Would you like to use this new key as your personal key for <email>?"

Comment 88

[Magnus Melin [:mkmelin] (away, back June 30)]

I think if it's the first key (no other keys set), it should be ok to just set that for use after it's first created. There's really little chance the user would NOT want it in that case, and if he really does, then he can select none.

Comment 89

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9157545 [details] [diff] [review]
1627736-openpgp-setup.diff

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

I think it works quite nicely. I'd like us to fix the fingerprint notation to consistently use the 0x notation, and I do have some thoughts on some of the wordings, but we can take that later.

For the things not implemented, you could just add a simple alert like alert("WIP: revocation not yet implemented");
That way it would be clearer what's WIP.

::: mail/extensions/openpgp/content/strings/key-wizard.ftl
@@ +104,5 @@
> +## Import Key section
> +
> +openpgp-import-key-title = Import existing OpenPGP Key
> +
> +

extra blank line here

::: mail/extensions/openpgp/content/ui/keyWizard.js
@@ +49,5 @@
> +// will be published to others who may not know the native language of the user.
> +const revocationFilePrefix1 =
> +  "This is a revocation certificate for the OpenPGP key:";
> +const revocationFilePrefix2 = `
> +A revocation certificate is a kind of "kill switch" to publicly

kind of *a*

::: mail/extensions/openpgp/content/ui/keyWizard.xhtml
@@ +101,5 @@
> +                   data-l10n-id="radio-keygen-expiry"
> +                   oncommand="onExpirationChange(event);"/>
> +            <html:input id="expireInput" type="number"
> +                        class="size4 input-inline autosync"
> +                        size="5" maxlength="5" value="1"

input type="number" doesn't do size, you just have to use css. (E.g. class="size5")

Comment 90

[Magnus Melin [:mkmelin] (away, back June 30)]

(Bugzilla doesn't really like reviews anymore... commenting on one thing will clear other flags)

Comment 91

[Richard Marti (:Paenglab)]

Comment on attachment 9157545 [details] [diff] [review]
1627736-openpgp-setup.diff

Looks good UI wise with nits that can be fixed in follow-ups.

Comment 92

[Richard Marti (:Paenglab)]

Attachment: Key-property-dialog.png

The Key Properties dialog has some glitches.

With dark system theme the fields don't switch to white on dark.
In the Structure tab many columns are cropped.
The Key Part column has a gap on the left.

Comment 93

[Alessandro Castellani [:aleca]]

Thanks for the positive review and feedback, I'm quickly updating the things you pointed out:

UI

• Increase the size of the Key Details dialog to fix everything in the tree.
• Added missing CSS to properly style inputs.

Richard, I'm not able to figure out why the Key Part column has a gap on the left, it's seems to be a CSS inherited from somewhere, would you be able to investigate?

• I'm adding the 0x prefix to all the keys used in the UI, only the strings not the actual values to not interfere with the implementation.
• I'm adding alert() for all those methods not yet implemented.
• I'm adding a small inline warning for the Import keys.
• I'm autoselecting the newly generated key after the wizard is closed.
• Autoselect None if the openpgp_key_id attribute is absent in the identity.
• Fix the UI reloading issue when switching between 2 email accounts.

Magnus, do you want to go through the edits you'd like to do to those strings?
I can fix those and then we can land this tomorrow morning if it looks good.

Comment 94

[Magnus Melin [:mkmelin] (away, back June 30)]

Let's think about the strings later.

Comment 95

[Richard Marti (:Paenglab)]

Comment on attachment 9157545 [details] [diff] [review]
1627736-openpgp-setup.diff

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

::: mail/extensions/openpgp/content/ui/keyDetailsDlg.xhtml
@@ -114,5 @@
>  
>            <treecols>
>              <treecol id="sig_uid_col" flex="1"
>                       data-l10n-id="openpgp-key-details-uid-certified-col"
>                       primary="true"/>

The gap comes from here. When I remove the `true`, the gap goes away. Probably it is the place for the twisty like we have for the threads in the main thread pane.

Comment 96

[Richard Marti (:Paenglab)]

See comment 95.

Comment 97

[Alessandro Castellani [:aleca]]

Attachment: 1627736-openpgp-setup.diff

This should address all the issues listed above.

Since we're not currently covering the OpenPGP implementation with tests, a try-run sounds a bit silly, but I launched one just to be sure I didn't accidentally bust any test related to e2e.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6054a0ed3e020497659726d6dc9df6985d47a0f0

Comment 98

[Magnus Melin [:mkmelin] (away, back June 30)]

Comment on attachment 9157812 [details] [diff] [review]
1627736-openpgp-setup.diff

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

Try looks good, so I'd go ahead and land this an iterate further (maybe with the WIP strings just hardcoded, or as a followup)

::: mail/extensions/openpgp/content/strings/enigmail.ftl
@@ +239,5 @@
> +  .tooltiptext = More information
> +
> +openpgp-key-revoke-title = Revoke Key
> +
> +openpgp-key-revoke-description = Work in Progress: Key revocation not yet implemented

I think we don't want these notifications localized - users shouldn't really see them. We don't want them mixed up there when we actually start localizing...

Comment 99

[Richard Marti (:Paenglab)]

Comment on attachment 9157812 [details] [diff] [review]
1627736-openpgp-setup.diff

Thanks. On the Key Properties dialog html:input fields you should add a `type="text"` to get the correct colour with the dark theme.

Comment 100

[Alessandro Castellani [:aleca]]

Attachment: 1627736-openpgp-setup.diff

Nits fixed.
Kai, we're landing this and continue the work on a follow up bug, so if you find any issues, please let me know and I'll address them immediately.

Comment 101

[Alessandro Castellani [:aleca]]

Comment on attachment 9157934 [details] [diff] [review]
1627736-openpgp-setup.diff

[Approval Request Comment]
Regression caused by (bug #): no regression
User impact if declined: important UI and UX improvements for the OpenPGP implementation
Testing completed (on c-c, etc.): not yet
Risk to taking this patch (and alternatives if risky): low as the original methods haven't been touched

Comment 102

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0d3a2130afc6
Improve the UI and UX of the OpenPGP configuration workflow. r=mkmelin

Comment 103

[Wayne Mery (:wsmwk)]

Comment on attachment 9157934 [details] [diff] [review]
1627736-openpgp-setup.diff

Approved for beta

Comment 104

[Rob Lemley [:rjl]]

That did not merge, I am guessing that Kai has other bugs to uplift for b3 that need to go first.

Comment 105

[Kai Engert [:KaiE:]]

(In reply to Rob Lemley [:rjl] from comment #104)

That did not merge, I am guessing that Kai has other bugs to uplift for b3 that need to go first.

Step 1:

• land https://bug1638822.bmoattachments.org/attachment.cgi?id=9156726 once approved

Step 2:

• merge this patch after changes from bug 1639107, only file mail/extensions/am-e2e/am-e2e.js needs merging

Comment 106

[Kai Engert [:KaiE:]]

Attachment: 1627736-beta78.patch

merged version of patch (only change is inside removed code)

Comment 107

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/releases/comm-beta/rev/ba230614672e3f9ca567e11c4dec35d12bbd2882

Comment 108

[Kai Engert [:KaiE:]]

I saw someone on Twitter claiming that importing of existing secret keys isn't yet supported by TB 78.
This is incorrect.
I believe the person made that incorrect conclusion, because the easy to find UI are the e2e prefs, and there, the import button says "not yet implemented".
It would be better to say something like "please use the OpenPGP Key Manager to import keys".

Comment 109

[Alessandro Castellani [:aleca]]

That missing feature in the UI will be fixed in bug 1647023

Comment 110

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #109)

That missing feature in the UI will be fixed in bug 1647023

Sure, but you'll need some more time. My point is, people who start experimenting, will falsely assume they cannot yet import keys at all, therefore the "not yet implemented" is misleading.

I suggest that we change the message for 78.0, until you have implemented import in prefs.

Comment 111

[Kai Engert [:KaiE:]]

Attachment: 1627736-import-notyet.patch

Clarify that key importing is supported, but needs to be done elsewhere.

Comment 112

[Kai Engert [:KaiE:]]

Comment on attachment 9160302 [details] [diff] [review]
1627736-import-notyet.patch

Clarify an OpenPGP work-in-progress text, to help testers discover an existing functionality.

Comment 113

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d47cfd3da956
Clarify key import work-in-progress text. r=mkmelin DONTBUILD

Comment 114

[Wayne Mery (:wsmwk)]

Comment on attachment 9160302 [details] [diff] [review]
1627736-import-notyet.patch

Approved for 78

Comment 115

[Rob Lemley [:rjl]]

(In reply to Wayne Mery (:wsmwk) from comment #114)

Comment on attachment 9160302 [details] [diff] [review]
1627736-import-notyet.patch

Approved for 78

Should this go on 79b1 as well?

Comment 116

[Wayne Mery (:wsmwk)]

Comment on attachment 9160302 [details] [diff] [review]
1627736-import-notyet.patch

Approved for 79 beta after discussion with Rob

Comment 117

[Rob Lemley [:rjl]]

Thunderbird 78.0:
https://hg.mozilla.org/releases/comm-esr78/rev/9ef9993d3a44

Comment 118

[Rob Lemley [:rjl]]

Thunderbird 79.0b1:
https://hg.mozilla.org/releases/comm-beta/rev/cbf404ed0459