We should make a few changes to the display of expired keys in the account preferences (introduced with bug 1627736).

(However, let's postpone this 2-3 weeks, until we have some overlapping code work done.)

I have several expired keys (from testing), which can lead to a long list of keys.

Requests:

• limit the number of expired keys we display, maybe 3. The primary intention is to offer the user easy access to the most recently expired keys, to decide which one to renew.
• sort the list of keys. The non-expired keys should be shown on top. The remaining keys should follow by date of expiry, most recently expired first.
• selecting an expired key should be prevented (disable the radio button). However, working with other elements of the key should remain possible (to view details and extend it). Note that at the time you open the dialog, the currently selected entry might already be expired. This should be possible. The item would be selected, but disabled.

This is somewhat related to the work I'm currently doing for bug 1647023

I'm not sure this is a problem in real usage. You don't really want to end up in a situation with 10s of expired keys.
We have plenty of room to scroll too if it's needed, so I would simply avoid this complication.

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

I'm not sure this is a problem in real usage. You don't really want to end up in a situation with 10s of expired keys.
We have plenty of room to scroll too if it's needed, so I would simply avoid this complication.

I agree with this.
We can assume that your situation is pretty unique Kai, and if a user ends up with lots of expired keys, he can delete them if he wants.

I'll take care of properly sorting the keys by expiration date.

I see in the bug 1647023 you already ensured that expired items cannot be selected, good.
I'm ok to keep all expired keys visible for now.

So the remaining action is to sort the list.
Do you want to do that as part of bug 1647023 ? Then you could mark this one as a dupe of it.

Nice.
I was working on the sorting patch on top of bug 1647023 while waiting for a review.
I prefer to keep things separate.
I'll upload the follow up patch here since that bug got an r+

This patch applies on top of bug 1647023.

This patch takes care of sorting keys by the most close to expire to the least.
It also pushes the keys that don't expire at the bottom of the list.

There was the initial requirement to push Expired keys at the bottom of the list, but I don't think it's correct.
I understand the need to move the expired keys "out of the ways", but I think this necessity is only applicable to us that we're testing and we're dealing with multiple keys.

Since we're ordering keys by date, it feels natural to have keys with a smaller date in first position.
Furthermore, expired keys should have the spotlight and be immediately visible to the user.

Is there any particular scenario where users might need to keep expired keys indefinitely, without extending the expiration date or deleting them?

return (
  !a.expiryTime - !b.expiryTime ||
  -(new Date(b.expiryTime) - new Date(a.expiryTime))
);

I don't understand this.

The first condition checks if one of the 2 key we're checking doesn't have an expiration date.
Since the "no expiration" is stored as value 0, without this condition those keys would always be listed in first position.
With this condition we have a 0 if both keys have expiration, or 1/-1 if one doesn't, sorting them accordingly.

The second condition is to account for a value of 0 in the first condition, meaning both keys have expiration dates.
We subtract the difference between the 2 dates to see if we have a negative or positive value in order to sort them.

Upon further testing, I think it's not necessary to create dates from those timestamps, and I can do a simple a - b instead of -(b - a).

I guess I can simplify it with this:
return !a.expiryTime - !b.expiryTime || a.expiryTime - b.expiryTime;

And please add a code comment about expiryTime 0 meaning not expiring.
Though I do like condense code it could be clearer to have

if (a.expiryTime === 0) {
return....

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

Non-expiring keys should be first, not last.

Depends on the intention of this list.
The purpose of that ordering is to show the near expiring keys first and give those importance.
Non-expiring keys don't require any action, therefore they don't need to be under the spotlight.

I'm okay with either solution, but we should define the purpose of the default sorting.

• Showing first the keys that the user should use since they don't expire.
• Showing the near expiring keys first to grab the user's attention on managing those keys.

The primary scenario needing attention is, if the user currently has a key configured, and this configured key is nearing expiration (or has already expired).

If there is already a selected key, and if there are additional keys that are expiring (or have already expired), the user most likely doesn't care about those additional keys (because the user is using a different key).

If you want to ensure attention for the most important scenario, I'd always list the currently selected key as the topmost entry.

If you want to avoid that the user gets unnecessarily confused by extra keys (which the user might have accidentally created and not yet deleted), I'd limit the amount of expired keys that are shown (e.g. to 3) as I've previously suggested.

Regarding expiring keys or not expiring keys, it's difficult to say which key is more important. Another attribute might be more relevant for this decision.

Most likely, the most recently created key has the highest likelihood to be user's desired choice as their selected key. If you'd like to base the order on that criteria, then sort by the keyCreated attribute (highest value is most recently created, sort descending).

The sorting of the keys shouldn't change based on the selection. Doing so will lead to an inconsistent UI with elements changing order without control.
A default sorting should always remain consistent, regardless the user's selection.

If you want to avoid that the user gets unnecessarily confused by extra keys (which the user might have accidentally created and not yet deleted), I'd limit the amount of expired keys that are shown (e.g. to 3) as I've previously suggested.

Not recommended as we would be hiding personal keys, forcing the user to go into the Key manager and find them. Not intuitive and breaking the simple visual separation we're trying to achieve, which is listing the user's personal keys all in one place for easy control.

Most likely, the most recently created key has the highest likelihood to be user's desired choice as their selected key.

I'd be okay with sorting them by creation date, but we need to stick with it and keep it consistent.
We shouldn't change the order based on other parameters.

Well the most important, and likely only important key is the one that's selected for usage. You're not supposed to be maintaining multiple keys for the same address. If you have a non-expiring key, I think that should be first since it's the one you would use.

Purely using creation date, newest on top seems like a good way to go.

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

Purely using creation date, newest on top seems like a good way to go.

Roger roger!

[Approval Request Comment]
User impact if declined: Inconsistent sorting of available keys
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): very low as it only sorts the keys

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/69366c1590ea
Sort OpenPGP key list by creation date. r=mkmelin

https://hg.mozilla.org/releases/comm-esr78/rev/ccec01e23248b2a3603b89096dcb836ea9a66513

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