Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
Daniel Kahn Gillmor writes: > When we have not been able to evaluate the signature status of a given > MIME part, showing a content-free (and interaction-free) "[ Unknown > signature status ]" button doesn't really help the user at all, and > takes up valuable screen real-estate. > > A visual reminder that a given message is *not* signed isn't helpful > unless it is always present, in which case we'd want to see "[ Unknown > signature status ]" buttons on all messages, even ones that don't have > a signing structure, but i don't think we want that. amended version pushed to master d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
On Fri 2019-05-24 22:38:12 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote: >>> Daniel Kahn Gillmor writes: >>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el index 353f721e..68171153 100644 --- a/emacs/notmuch-crypto.el +++ b/emacs/notmuch-crypto.el @@ -93,6 +93,7 @@ mode." (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) (let* ((status (plist-get sigstatus :status)) (help-msg nil) + (show-button t) (label "Signature not processed") >>> >>> This should probably be nil, since that particular value is never used, >>> iiuc. I can amend it if you agree. >> > [snip] >> >> If i've misunderstood the e-lisp (entirely possible!) i would be happy >> to be corrected. > > No, you've just misunderstood my reply. I refer to the existing initialization > of "label", which now seems obsolete to me. Ah, sorry! i understand now, and yes, i agree that label shold be initialized to nil. If you're willing to amend it that sounds good to me. Thanks for catching this. --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
Daniel Kahn Gillmor writes: > On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote: >> Daniel Kahn Gillmor writes: >> >>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el >>> index 353f721e..68171153 100644 >>> --- a/emacs/notmuch-crypto.el >>> +++ b/emacs/notmuch-crypto.el >>> @@ -93,6 +93,7 @@ mode." >>> (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) >>>(let* ((status (plist-get sigstatus :status)) >>> (help-msg nil) >>> +(show-button t) >>> (label "Signature not processed") >> >> This should probably be nil, since that particular value is never used, >> iiuc. I can amend it if you agree. > [snip] > > If i've misunderstood the e-lisp (entirely possible!) i would be happy > to be corrected. No, you've just misunderstood my reply. I refer to the existing initialization of "label", which now seems obsolete to me. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el >> index 353f721e..68171153 100644 >> --- a/emacs/notmuch-crypto.el >> +++ b/emacs/notmuch-crypto.el >> @@ -93,6 +93,7 @@ mode." >> (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) >>(let* ((status (plist-get sigstatus :status)) >> (help-msg nil) >> + (show-button t) >> (label "Signature not processed") > > This should probably be nil, since that particular value is never used, > iiuc. I can amend it if you agree. I don't think i understand your suggestion. that value *is* used, by the "(when show-button …)" construct later in the code. The idea was to make the button shown by default, unless it was disabled further down in the patch (in the "t" clause of the main "cond" construct in notmuch-crypto-insert-sigstatus-button, which only triggers if we legitimately don't have any status information (if "status" is nil)). So no, i think it's supposed to start off "t" and then get disabled in the specific condition this patch tests for. If i've misunderstood the e-lisp (entirely possible!) i would be happy to be corrected. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
Daniel Kahn Gillmor writes: > diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el > index 353f721e..68171153 100644 > --- a/emacs/notmuch-crypto.el > +++ b/emacs/notmuch-crypto.el > @@ -93,6 +93,7 @@ mode." > (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) >(let* ((status (plist-get sigstatus :status)) >(help-msg nil) > + (show-button t) >(label "Signature not processed") This should probably be nil, since that particular value is never used, iiuc. I can amend it if you agree. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote: > When we have not been able to evaluate the signature status of a given > MIME part, showing a content-free (and interaction-free) "[ Unknown > signature status ]" button doesn't really help the user at all, and > takes up valuable screen real-estate. I haven't heard any public complaints about this proposed change, and it is a concrete improvement to the usability of encrypted-but-unsigned messages. Please consider merging! --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
On Mon 2019-04-22 13:26:05 -0400, Daniel Kahn Gillmor wrote: > On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote: >> When we have not been able to evaluate the signature status of a given >> MIME part, showing a content-free (and interaction-free) "[ Unknown >> signature status ]" button doesn't really help the user at all, and >> takes up valuable screen real-estate. >> >> A visual reminder that a given message is *not* signed isn't helpful >> unless it is always present, in which case we'd want to see "[ Unknown >> signature status ]" buttons on all messages, even ones that don't have >> a signing structure, but i don't think we want that. > > This is a small step down the path of making notmuch-emacs friendlier > with regards to encrypted messages, but it's one that will have an > effect on future patch series that work with encrypted messages. I'd be > happy to hear any concerns people have about this change, but i find > notmuch-emacs is more pleasant to use this way. I've heard from some people that they don't like this final patch because the "[ Unknown signature status ]" button is at least an indication that the message appears to be signed (even if we decided not to -- or were unable to -- evaluate the signature). I considered argument this when writing the patch initially, and i don't think it's a good argument for two reasons: a) without actual signature verification, the user experience is trivially scammable by an adversary who knows how to craft a MIME message, and it's basically encouraging a user pattern that is something along the lines of: https://xkcd.com/1181/ (though maybe a bit more subtle, based on MIME structure instead of the inline-signing that xkcd is mocking) This is a particularly bad security indicator and user experience. The thing isn't reliable, and it's not actionable in most cases. b) In the current state of the codebase, the presence of the button does *not* indicate that a signature-like thing is even present. If you look at test/emacs-show.expected-output/notmuch-show-decrypted-message, that shows the cleartext view of a decrypted message which *does not* have an OpenPGP signature on it at all (test/corpora/crypto/basic-encrypted.eml is encrypted but unsigned). I could imagine changing notmuch to fix concern (b) -- that is, hiding the button just in the case where no signature-looking thing is present at all. But i haven't seen anyone even identify that problem publicly yet, let alone offer a fix for it. But i think that (a) is at least as big of a concern as (b); the fix i'm proposing in this series is actually simpler than such a targeted fix would be; and the fix in this series actually solves both problems. If someone wants to offer a fix just for (b) on top of the first two patches in this series, i'd happily advocate for it as better than the status quo, which would let us put off (a) to a more interesting and targeted discussion. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
The first two patches in this series should be uncontroversial (they're just establishing a baseline), and i'd appreciate it if they could be considered for merging regardless of whether anyone raises an objection to this last patch of the series. On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote: > When we have not been able to evaluate the signature status of a given > MIME part, showing a content-free (and interaction-free) "[ Unknown > signature status ]" button doesn't really help the user at all, and > takes up valuable screen real-estate. > > A visual reminder that a given message is *not* signed isn't helpful > unless it is always present, in which case we'd want to see "[ Unknown > signature status ]" buttons on all messages, even ones that don't have > a signing structure, but i don't think we want that. This is a small step down the path of making notmuch-emacs friendlier with regards to encrypted messages, but it's one that will have an effect on future patch series that work with encrypted messages. I'd be happy to hear any concerns people have about this change, but i find notmuch-emacs is more pleasant to use this way. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
When we have not been able to evaluate the signature status of a given MIME part, showing a content-free (and interaction-free) "[ Unknown signature status ]" button doesn't really help the user at all, and takes up valuable screen real-estate. A visual reminder that a given message is *not* signed isn't helpful unless it is always present, in which case we'd want to see "[ Unknown signature status ]" buttons on all messages, even ones that don't have a signing structure, but i don't think we want that. --- emacs/notmuch-crypto.el | 27 ++- .../notmuch-show-decrypted-message| 1 - .../notmuch-show-decrypted-message-no-crypto | 1 - ...notmuch-show-process-crypto-mime-parts-off | 1 - .../notmuch-show-undecryptable-message| 1 - 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el index 353f721e..68171153 100644 --- a/emacs/notmuch-crypto.el +++ b/emacs/notmuch-crypto.el @@ -93,6 +93,7 @@ mode." (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) (let* ((status (plist-get sigstatus :status)) (help-msg nil) +(show-button t) (label "Signature not processed") (face 'notmuch-crypto-signature-unknown) (button-action (lambda (button) (message (button-get button 'help-echo) @@ -118,19 +119,21 @@ mode." (let ((keyid (concat "0x" (plist-get sigstatus :keyid (setq label (concat "Bad signature (claimed key ID " keyid ")")) (setq face 'notmuch-crypto-signature-bad))) + (status + (setq label (concat "Unknown signature status: " status))) (t - (setq label (concat "Unknown signature status" - (if status (concat ": " status)) -(insert-button - (concat "[ " label " ]") - :type 'notmuch-crypto-status-button-type - 'help-echo help-msg - 'face face - 'mouse-face face - 'action button-action - :notmuch-sigstatus sigstatus - :notmuch-from from) -(insert "\n"))) + (setq show-button nil))) +(when show-button + (insert-button + (concat "[ " label " ]") + :type 'notmuch-crypto-status-button-type + 'help-echo help-msg + 'face face + 'mouse-face face + 'action button-action + :notmuch-sigstatus sigstatus + :notmuch-from from) + (insert "\n" (declare-function notmuch-show-refresh-view "notmuch-show" (&optional reset-state)) diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message b/test/emacs-show.expected-output/notmuch-show-decrypted-message index 08a9e4f6..c4ec4b32 100644 --- a/test/emacs-show.expected-output/notmuch-show-decrypted-message +++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message @@ -5,7 +5,6 @@ Date: Sat, 01 Jan 2000 12:00:00 + [ multipart/encrypted ] [ Decryption successful ] -[ Unknown signature status ] [ application/pgp-encrypted ] [ text/plain ] The password is "abcd1234!", please do not tell anyone. diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto index e302e452..49d85337 100644 --- a/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto +++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto @@ -5,6 +5,5 @@ Date: Sat, 01 Jan 2000 12:00:00 + [ multipart/encrypted ] [ Unknown encryption status ] -[ Unknown signature status ] [ application/pgp-encrypted ] [ application/octet-stream ] diff --git a/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off b/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off index ce2892a0..3282c7b1 100644 --- a/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off +++ b/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off @@ -9,7 +9,6 @@ Subject: [notmuch] Working with Maildir storage? [ multipart/mixed ] [ multipart/signed ] - [ Unknown signature status ] [ text/plain ] > See the patch just posted here. diff --git a/test/emacs-show.expected-output/notmuch-show-undecryptable-message b/test/emacs-show.expected-output/notmuch-show-undecryptable-message index 530ff286..95c1646b 100644 --- a/test/emacs-show.expected-output/notmuch-show-undecryptable-message +++ b/test/emacs-show.expected-output/notmuch-show-undecryptable-message @@ -5,6 +5,5 @@ Date: Thu, 22 Dec 2016 08:34:56 -0400 [ multipart/encrypted ] [ Decryption error ] -[ Unknown signature status ] [ application/pgp-encrypted ] [ application/octet-stream ] -- 2.20.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
