1708914 - OpenPGP After changing key expiration date, UI still shows old date

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Kai Engert [:KaiE:]]

We fail to update the UI after changing the expiration date of a key.

(1)
Expiration information in key details not updated - need to reopen dialog.

Open OpenPGP key manager.
Doubleclick a personal key, viewing the details.
Change expiration date.
You are back in the details dialog.

Bug: The dialog still shows the old date.

Close the details dialog. Look for this key in the key manager (which is still shown).
You'll note the expiration date has (correctly) already been updated.

Expected: The key details dialog should immediately be updated to show the new date after returning from the expiration change dialog.

(2)
Expiration information in E2E account settings not updated, after having edited expiration using key manager - need to reopen tab.

Prior to performing the actions described in (1), open account settings. Find the email account that is related to the key you are going to extend. Open the E2E tab, and see the old expiration date.

Now perform the actions described in (1). After you are done with (1), close the openpgp key manager.
Note that here in account settings, the old expiration date is still shown.
It's necessary to close the account settings tab and reopen it, then the new date will be shown.

(3)
Expiration information in E2E account settings not updated, after having edited expiration using account settings - need to restart application.

Open account settings, E2E tab.
Click on a button that offers to change an expiration date.
Confirm to extend the expiration.
You're back in the E2E account settings tab.

Bug a: The old expiration date is still shown.

Close the tab. Reopen it.
The old expiration date is still shown.

Open OpenPGP key manager.
The old expiration date is still shown.

Restart Thunderbird.
Finally, the new expiration date is shown.

Comment 1

[Kai Engert [:KaiE:]]

Magnus had reported that changing the expiration date isn't working.
It seems the feature is actually working, but the UI not updating gives this false impression.

Comment 2

[Magnus Melin [:mkmelin]]

If the key was originally set to not expire, it doesn't work at all.
For keys that have a date, one can't shorten it.
And playing around with this, changing expiry date for the key with a date it actually created a new key (bad!!).

Comment 3

[Magnus Melin [:mkmelin]]

For "never expire" perhaps bug 1720008.

Comment 4

[SnipFoo]

Attachment: Bug 1708914 - Wait for asynchronous call to return in changeExpiryDlg. r=mkmelin

Comment 5

[SnipFoo]

Hello,

I can confirm bug (1). I couldn't reproduce bugs (2) and (3), however.

For bug (1), from what I could gather from my investigations, it seems that the dialog changeExpiryDlg closes before the call to the asynchronous function onAccept() finishes: as can be seen on ui/changeExpiryDlg.js, line 144, the result of the call is not handled as a Promise, but as a regular boolean. (Note that the onAccept() function is in charge of updating the key with the new expiration date in the keyring.)

When closing, the dialog calls a callback function (window.arguments[0].modified) for updating the parent dialog (see ui/changeExpiryDlg.js, line 149), which itself refreshes the cache of OpenPGP keys (see e.g., ui/keyDetailsDlg.js, line 119 for such a function). This leads to a race condition, as onAccept() doesn't have time to update the keyring before the cache is refreshed for the parent dialog.

I've tried to fix this by having the handler for the dialogaccept event wait for onAccept() to return before actually closing the dialog. With this method, the event handler is in fact called twice:

• The first time around (when the OK button is clicked, for instance), gDone is false, which prompts the handler to call the asynchronous function onAccept() to update the keyring, and to set up a callback function for when the corresponding Promise is resolved.
• Upon resolution, if onAccept() returned true, this callback sets gDone to true and triggers a second dialogaccept event for the dialog.
• This second time around, the dialogaccept event handler calls the parent-updating callback, then properly closes the dialog.

I don't know if this is the proper way to do this, but from the tests I could run, this modification fixed the bug: the changeExpiryDlg dialog waits for the keyring update before closing, and the parent dialog correctly shows the new expiration date.

I've just submitted this as a patch: https://phabricator.services.mozilla.com/D142397. I hope that this will help!

Thank you, and thanks to Magnus Melin for pointing me to this bug! :)

SnipFoo.

Comment 6

[Magnus Melin [:mkmelin]]

The patch fixes #1, but I still see #2, which will need a similar/same solution as bug 1760150. #3 I don't see at least with the patch applied.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/62e9dcf14879
Wait for asynchronous call to return in changeExpiryDlg. r=mkmelin