1766906 - Enable UI for "acceptance per email" if necessary for repairing. Disable ok button if no email is checked.

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert (:KaiE:)]

Currently, the recently added UI in bug 1755281 is only shown when it seems useful. If a key only has one email address, one might think that it isn't useful.

However, I just experienced a data inconsistency in my own Thunderbird profile.

I had a key marked as accepted, but there wasn't any record for an accepted email address.

This surprises me. Our code uses transactions that should carefully avoid this state. I don't know how this state could occurr in my profile. Maybe it was a bug in one of the versions I tested?

I think it may be useful to accept the UI from bug 1755281 unconditionally for all keys, even if they have just a single email address. This can allow for repairing of such a state.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Bug 1766906 - Enable UI for "acceptance per email" if necessary for repairing. Disable ok button if no email is checked. r=mkmelin

Depends on D145008

Comment 2

[Magnus Melin [:mkmelin]]

I don't know... if it's for a state that never should occur, we might not want to show it, as it can also cause needless confusion.
To repair without it, toggling the main acceptance would fix the situation no? And with the key assistant, that would also solve such a problem directly I think.

Comment 3

[Kai Engert (:KaiE:)]

You're mostly correct.

However, if our data really arrives in that state, the user has no way to see the cause.

Ok, here's an idea for an improvement, to address your concern:

Let's show and enable this tab ONLY in the broken scenario - if the key is accepted, but no email address is marked as accepted.

Comment 4

[Magnus Melin [:mkmelin]]

Sounds good

Comment 5

[Kai Engert (:KaiE:)]

Alex, we need your guidance for the best behavior of the "key details" dialog, regarding ok/close and cancel buttons.

That's the dialog that is shown when you view the details of an OpenPGP key in various places, such as the OpenPGP key manager.

Today, we only have an "OK" button, we don't show a cancel button.
I don't remember exactly why we made it that way.

I proposed to add a Cancel button, but we're not sure yet if that's the correct thing to do.

Today, the dialog contains the following ways to make changes:
(1) click one of the acceptance choices
(2) click the email checkboxes
(3) for a personal key, use the "change expiry" button to edit the expiration property in a sub-dialog

Today, if you change (1) or (2), the changes are only on screen. They will be saved after the user clicks OK.

For (3), if the expiration subdialog is confirmed, the change is immediately active, and the user is brought back to this key details dialog.

If we add a Cancel button, might the user be confused? Might the user incorrectly conclude that clicking cancel will also revert the changes made in the expiration subdialog?

If no, great, then we could add the cancel button. However, note that in the future, we might have to add additional actions like (3) to the dialog (for example, when editing other properties of the key).

If yes, should we change the behavior of the dialog? Should we immediately save, for each click on an acceptance choice or on an email checkbox? That probably means we'd have to rename the OK button to save "Close".

Thanks in advance for your input.

Comment 6

[Kai Engert (:KaiE:)]

My personal preference is to keep the current behavior (delay saving of choices) and add an OK button.

The reason is, I want to disable saving/ok, if the user unchecks all email checkboxes.

If you ask us for immediate saving, I'd have to prevent the user from unchecking all checkboxes. (We would have to disable the email checkboxes if only one checkbox is checked.)

Comment 7

[Magnus Melin [:mkmelin]]

If you ask us for immediate saving, I'd have to prevent the user from unchecking all checkboxes. (We would have to disable the email checkboxes if only one checkbox is checked.)

Well, that could/should then move the key back to not accepted and everything is fine.

Comment 8

[Kai Engert (:KaiE:)]

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

If you ask us for immediate saving, I'd have to prevent the user from unchecking all checkboxes. (We would have to disable the email checkboxes if only one checkbox is checked.)

Well, that could/should then move the key back to not accepted and everything is fine.

ok, it might be a bit unexpected, but we could do that.

Comment 9

[Alessandro Castellani [:aleca]]

Thanks for the question.

I think having the "Cancel" button in the Key Details dialog it's fine and that action should NOT save any changes the user has done in that dialog.

Regarding the "Change Expiration Date", I don't think that applies because that button opens another dialog where the users will need to accept and close in order to save the changed date.
That means the Ok and Cancel of the expiration date change dialog are different from the Ok and Cancel of the Key Details dialog.

I think it's acceptable to keep the current behavior (and add that if you click Cancel we don't save what you changed for 1) and 2)), and it shouldn't be confusing for the users.

Comment 10

[Kai Engert (:KaiE:)]

Thanks Alex!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/916e05054a51
Enable UI for "acceptance per email" if necessary for repairing. Disable ok button if no email is checked. r=mkmelin

Comment 12

[Kai Engert (:KaiE:)]

Comment on attachment 9274357 [details]
Bug 1766906 - Enable UI for "acceptance per email" if necessary for repairing. Disable ok button if no email is checked. r=mkmelin

I'd like to have beta testing of this code.

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9274357 [details]
Bug 1766906 - Enable UI for "acceptance per email" if necessary for repairing. Disable ok button if no email is checked. r=mkmelin

[Triage Comment]
Approved for beta

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 101.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/11b4e748bc91