1832369 - Truncated OK button in OpenPGP `Success! Keys imported` dialog

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Thomas D. (:thomas8)]

Attachment: Screenshot 1: `Success! Keys imported` dialog with truncated OK button

+++ This bug was initially created as a clone of Bug #1831790 +++

Seen on Daily 115.0a1 (2023-05-09) (64-bit), Win10, or well, a couple of days earlier.

STR:

1. Compose new message from OpenPGP enabled account
2. Press Encrypt button in composition toolbar so that Encrypt+OpenPGP are active
3. Add recipient whose public key you haven't imported yet.
4. From yellow notification bar "End-to-end encryption requires resolving key issues...", click Resolve
5. On OpenPGP Key Assistant dialog, click Import Public Keys From File...
6. Select a public key file and confirm that
7. Check the layout of the resulting little Success: Keys imported dialog wrt OK button.

Actual:

• OK button is truncated at the bottom, dialog too small

Expected:

• Show all of OK button

Probably a regression from bug 1820744, flexbox changes.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: Bug 1832369 - Remove key imports dialog in favor of a normal alert. r=kaie

The dialog in question was really atrocious. Just alert about what happened instead.
Open coding it so we can eventually get rid of EnigmailDialog.

Comment 2

[Kai Engert [:KaiE:]]

I agree that the old dialog was ugly, however, it offered the ability to view the key immediately, and potentially verify it, and immediately change its acceptance property.

Magnus, with your approach, you only show a short summary with the key ID, which makes it hard to do the above. If you have many keys, it isn't easy to find that key afterwards in the key manager.

Can you think of a way to extend your solution, by adding a "view key" button to that alert, which would then open a modal dialog, showing the keyDetailsDlg for that key?

Comment 3

[Magnus Melin [:mkmelin]]

My reasoning was this: in most scenarios all information about they key (keys) is shown on the prior import screen, which also includes the option to set as accepted or not. With that in mind, the need to take any action after seems somewhat questionable. Inspecting keys is a pretty weird thing to do for any normal user to begin with. Further to this, opening details from that is usually a 3rd layer of nested dialogs :/

Comment 4

[Kai Engert [:KaiE:]]

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

My reasoning was this: in most scenarios all information about they key (keys) is shown on the prior import screen, which also includes the option to set as accepted or not.

There is at least one scenario in which we don't show a prompt prior to importing - it's when silently updating a key, because we already have a found key, we learn it's a reasonable thing to automatically import the updated key. Then we don't ask for permission, we just import. In this scenario, it was good to have the ability to view the updated key with a click, to learn what's new.

I realize my previous suggestion from comment 2 doesn't work if we'd like to tell the user about more than 1 imported/updated key.

With that in mind, the need to take any action after seems somewhat questionable. Inspecting keys is a pretty weird thing to do for any normal user to begin with.

I think you're trying to say that only "experts" might want to view that information? I think that isn't a sufficient argument to removal, if the experts still want it.

Further to this, opening details from that is usually a 3rd layer of nested dialogs :/

Well, I'm open to suggestions to reduce the amount of modal dialog layering. But I think it must be possible to find a way to present information.

If we cannot stack another layer on top, we need a solution that has one level dialog, and dynamically replace the contents of that dialog - just like it's done in the key assistant dialog.

Comment 5

[Magnus Melin [:mkmelin]]

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

There is at least one scenario in which we don't show a prompt prior to importing - it's when silently updating a key, because we already have a found key, we learn it's a reasonable thing to automatically import the updated key. Then we don't ask for permission, we just import. In this scenario, it was good to have the ability to view the updated key with a click, to learn what's new.

Maybe there are multiple entry points, but I think we'd still be showing confirmation only as response to user action, and for that case you would normally be looking at the key in the dialog below.

I think you're trying to say that only "experts" might want to view that information? I think that isn't a sufficient argument to removal, if the experts still want it.

No, I think only for debugging purposes perhaps.

Well, I'm open to suggestions to reduce the amount of modal dialog layering. But I think it must be possible to find a way to present information.

I think the whole process would need some overhaul. But that's a larger project.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: Bug 1832369 - Rework the key import info details dialog. r=#thunderbird-front-end-reviewers,kaie

Rework the dialog to just a list of imported keys, with their ids.
The ids can be clicked to open the key ketails and check properties.

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: Bug 1832369 - Make key import info dialog buttons show. r=aleca

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8e7ded91a4f2
Make key import info dialog buttons show. r=aleca

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9340674 [details]
Bug 1832369 - Make key import info dialog buttons show. r=aleca

[Approval Request Comment]
Very safe regression fix

Comment 10

[Wayne Mery (:wsmwk)]

Comment on attachment 9340674 [details]
Bug 1832369 - Make key import info dialog buttons show. r=aleca

[Triage Comment]
Approved for beta

Comment 11

[Daniel Darnell [:dandarnell]]

Thunderbird 115.0b6:
https://hg.mozilla.org/releases/comm-beta/rev/0d8ba7ca065b