1599031 - Add ability to query build time option MOZ_OPENPGP from JS

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert (:KaiE:)]

While the openpgp integration is disabled by default, I need the ability to query from JS if MOZ_OPENPGP was enabled.

This is required for global messenger code, to enable/disable UI behavior.

(As a first step, I'll need that for the popup menu on the attachment list. Need that to test import of attached OpenPGP keys from received emails.)

Comment 1

[Kai Engert (:KaiE:)]

I'd like to implement something similar to FF AppConstants.

I see we already have mail/base/modules/MailConsts.jsm
but that doesn't use the "Object.freeze" wrapping that AppConstants uses for the build time flags.

Magnus, please let me know your preference:
(a) use the attached patch, to add an additional MailConstants module that uses Object.freeze
(b) add the new flag to the existing MailConsts.jsm and change that to also use Object.freeze

Comment 2

[Kai Engert (:KaiE:)]

Attachment: 1599031-v1.patch

Comment 3

[Kai Engert (:KaiE:)]

Hmm, I just notice that I copied the style that AppConstants used:

this.EXPORTED_SYMBOLS
...
this.MailConstants = Object.freeze({

I guess I should rather use the following?

var EXPORTED_SYMBOLS
...
var MailConstants = Object.freeze({

Comment 4

[Kai Engert (:KaiE:)]

Attachment: 1599031-v2.patch

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9111171 [details] [diff] [review]
1599031-v2.patch

Review of attachment 9111171 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, preferably these kind of build-time options should go into AppConstants.jsm, but we can't really touch that...

The way it's used, for now it might be easiest to just define a global variable in bond.js?

Comment 6

[Kai Engert (:KaiE:)]

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

The way it's used, for now it might be easiest to just define a global
variable in bond.js?

If MOZ_OPENPGP is undefined, then file bond.js isn't loaded. Your suggestion would require to always load the bond.js file, which I tried to avoid.

We need a property in a JS file that's always loaded, even if OpenPGP is disabled.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9111171 [details] [diff] [review]
1599031-v2.patch

Review of attachment 9111171 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, let's go with this. r=mkmelin

Comment 8

[Kai Engert (:KaiE:)]

Thanks Magnus. I'm removing the checkin-needed-tb keyword. I'll land the OpenPGP related patches while we're in the initial development phase. I might depend on landing patches in a certain order.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5629275db993
Protect keyring using an automatic password that's protected with the master password. r=patrick DONTBUILD
https://hg.mozilla.org/comm-central/rev/10abecc1ae80
Add ability to query build time option MOZ_OPENPGP from JS. r=mkmelin DONTBUILD

Comment 10

[Kai Engert (:KaiE:)]

(In reply to Pulsebot from comment #9)

https://hg.mozilla.org/comm-central/rev/5629275db993
Protect keyring using an automatic password that's protected with the master password. r=patrick DONTBUILD

This had an incorrect bug number in the commit message, it was for bug 1599233.

Comment 11

[Kai Engert (:KaiE:)]

backed out, reopening
https://hg.mozilla.org/comm-central/rev/beec6bc7dc921e59d67f4bd576b13d03f03e5c2a

Comment 12

[Jorg K (CEST = GMT+2)]

Please add a bug number to backout commit messages. hg qbackout does that automatically, not sure about hg backout.

Comment 13

[Jorg K (CEST = GMT+2)]

And was there any particular reason to land the backout without DONTBUILD as was agreed :-(

Comment 14

[Kai Engert (:KaiE:)]

Jörg, sorry for the missing bug number.

I think building was useful, because the build was busted, and needed to be fixed, and triggering a build to get a confirmation seemed appropriate.

The code that was backed out wasn't in the openpgp subdirectory. It was global integration code, which caused eslint to fail, and we weren't sure if it could have caused any other problems. Not sure why you're unhappy about an additional build.

Comment 15

[Jorg K (CEST = GMT+2)]

It was unnecessary. Despite the linting error, the Daily build ran through OK, so a backout wouldn't have changed the result. Since after
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=b8e5b419b13691d345f66b04ab3a936b9a0c4bea
the tree was pretty orange, there wasn't really a reason to repeat that. I don't know who cancelled the remaining jobs, but that was the correct action.

Comment 16

[Kai Engert (:KaiE:)]

Attachment: 1599031-v4b.patch

I found that all JS files that contain "#filter substitution" are listed in .eslintignore, even toolkit's AppConstants.jsm, so I'm adding our new MailConstants.jsm, too.

I've added
/* global Enigmail */
to mail/base/content/msgHdrView.js to fix the ESLint error.

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 9112379 [details] [diff] [review]
1599031-v4b.patch

Review of attachment 9112379 [details] [diff] [review]:
-----------------------------------------------------------------

This isn't a proper hg patch (missing commit message and such), but looks good otherwise

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c35cc8809fea
Add ability to query build time option MOZ_OPENPGP from JS. r=mkmelin DONTBUILD