1645914 - Add a preference to disable OpenPGP and hide the related UI

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert (:KaiE:)]

We need a preference to disable the OpenPGP functionality by default.
IMO the pref should also hide all related UI elements.

The pref will be necessary for 78.0 and 78.1, and might also help downstream distributors who wish to disable it.

We already have the MOZ_OPENPGP build time option.
However, we want to distribute builds that include the functionality, so we need a run time option.

Keeping the build time option might still be useful for downstreams who don't wish to provide the OpenPGP functionality, e.g. https://bugzilla.redhat.com/show_bug.cgi?id=1837512

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Bug 1645914 - Add a preference to disable OpenPGP and hide the related UI. r=mkmelin [checked in]

Comment 2

[Kai Engert (:KaiE:)]

The phab patch adds a preference, disables library loading if it's false, and also hides all UI elements related to OpenPGP.
Currently, a restart is required to get the effect of enabling the pref. I'd personally be happy with that requirement.

Magnus requested that we support runtime enabling, without requiring a restart .

Runtime enabling could fail, e.g. if the user tries it in an environment that doesn't distribute the RNP library.
On failure, it might be best to hide the UI elements, too.
However, that would require a two step approach for runtime enabling.
An initial observer would have to listen for the preference change, and should trigger the init attempt.
Only if init works, we'd broadcast a separate event, that enables the elements in all open windows.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3298f786eff2
Add a preference to disable OpenPGP and hide the related UI. r=mkmelin

Comment 4

[Kai Engert (:KaiE:)]

reopening to clarify the runtime enable details.

Comment 5

[Kai Engert (:KaiE:)]

Comment on attachment 9156853 [details]
Bug 1645914 - Add a preference to disable OpenPGP and hide the related UI. r=mkmelin [checked in]

required for 78 release

Comment 6

[Jorg K (CEST = GMT+2)]

Is this just supposed to disable the UI or the functionality as a whole? I noticed that the Enigmail-based OpenPGP implementation clashes with the pEp add-on left, right and centre. I had to nuke this so far ... and counting:

EnigmailVerify.registerContentTypeHandler - Causes bug 1646404
Enigmail.msg.sendMessageListener
Enigmail.msg.messengerStartup
Enigmail.hdrView.hdrViewLoad
Enigmail.hdrView.onShowAttachmentContextMenu

Basically all the stuff currently depending on MOZ_OPENPGP:
https://searchfox.org/comm-central/search?q=MOZ_OPENPGP&path=.js&case=false&regexp=false

EDIT: Oh, I see here https://hg.mozilla.org/comm-central/rev/3298f786eff2#l3.12 that the some of the stuff I mentioned above is taken care of, but not all, for example onShowAttachmentContextMenu here:
https://searchfox.org/comm-central/rev/9498b3d1accf12df749f19b88fb1d0a1fcad8d4d/mail/base/content/msgHdrView.js#2313

EDIT 2: Maybe onShowAttachmentContextMenu is take care of by the changes in attachmentItemContext.inc.xhtml, and sendMessageListener() doesn't get added in composeStartup() if OpenPGP isn't fully initialised. That leaves some code which runs EnigmailVerify.registerContentTypeHandler. Let me try this in a Daily.

Comment 7

[Jorg K (CEST = GMT+2)]

OK, sorry about the noise, just ignore my comment #6. In Daily with OpenPGP disabled I don't see bug 1646404. Looks like this
https://searchfox.org/comm-central/rev/9498b3d1accf12df749f19b88fb1d0a1fcad8d4d/mail/extensions/openpgp/content/BondOpenPGP.jsm#78
isn't called, so none of the handlers are initialised.

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9156853 [details]
Bug 1645914 - Add a preference to disable OpenPGP and hide the related UI. r=mkmelin [checked in]

Approved for beta

Comment 9

[Rob Lemley [:rjl]]

Thunderbird 78.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/5c08edff5bf2

Comment 10

[Kai Engert (:KaiE:)]

We did this.

Comment 11

[Jorg K (CEST = GMT+2)]

Sure, but you re-opened it in comment #4. It still needs a restart after flipping the pref, or am I missing something?

Comment 12

[Kai Engert (:KaiE:)]

Ok I had forgotten that we considered tracking it in this bug.
We actually did some work for runtime enabling in bug 1656287. But it's incomplete. See bug 1658603.