Closed
Bug 442730
Opened 16 years ago
Closed 15 years ago
composition security options for encrypt should only be one menu item (with checkbox) like signing
Categories
(Thunderbird :: Security, defect)
Thunderbird
Security
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0rc1
People
(Reporter: mkmelin, Assigned: mkmelin)
References
Details
(Whiteboard: [Bv1 is fixed-thunderbird3.1a1])
Attachments
(3 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
philor
:
review+
clarkbw
:
ui-review+
standard8
:
approval-thunderbird3+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mkmelin
:
review+
standard8
:
approval-thunderbird3.0.1-
|
Details | Diff | Splinter Review |
In the composition window, under Options > Security
[v] Do not encrypt this message
[ ] Encrypt this message
-------------------------------
[ ] Digitally sign this message
As you can see, the encryptiion has two entries, signing has one, which adds the checkmark when chosen.
S/MIME is confusing enough as it is... I don't see why there would have to be two menu items for one thing (for encryption). It should just be one, like signing.
Assignee | ||
Comment 1•15 years ago
|
||
I actually figured out why it's implemented like it is today - should be because ns4 had a "encrypt if possible" option, so encryption could be 3-state. However, that's not implemented (bug 135636) and the UI details of a good implementation of that are probably hard to get right anyway, especially as the cases where you'd have "always require encryption" are rare for real world scenarios. So i think we could worry about that in the future, and improve what we have for now.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #401700 -
Flags: ui-review?(clarkbw)
Attachment #401700 -
Flags: review?(philringnalda)
Assignee | ||
Comment 2•15 years ago
|
||
Screenshot with the patch applied.
Assignee | ||
Comment 3•15 years ago
|
||
Screeshot of the fix (for real this time!)
Attachment #401701 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Attachment #401702 -
Attachment is patch: false
Attachment #401702 -
Attachment mime type: text/plain → image/png
Comment 4•15 years ago
|
||
(In reply to comment #1)
> the UI details of a good implementation of that are probably hard to get
> right anyway, especially as the cases where you'd have "always require
> encryption" are rare for real world scenarios.
"always require encryption" should be the _normal_ encrypted state unless I'm completely misunderstanding what you're saying. If encryption is worth bothering with at all then sending some (unknown number of) copies unencrypted defeats the whole point.
The "encrypt if possible" abomination was an attempt to bootstrap in a world where signing certs were rare and hard to get. We should work to solve _that_ problem, not make it easier for that state to continue.
Comment 5•15 years ago
|
||
Comment on attachment 401700 [details] [diff] [review]
proposed fix
I may be misunderstanding the bug. Simplifying the options is good, I just hope it works like the menu makes it look like it works.
>+function toggleEncryptMessage()
...
>+ if (gSMFields.requireEncryptMessage)
...
>+ else
>+ {
>+ setNoSignatureUI();
>+ }
Shouldn't that be setNoEncryptionUI() ?
Assignee | ||
Comment 6•15 years ago
|
||
This bug doesn't change the functionality, i was just speculating about why it was done the way it is currently, and whether the UI should stay like it is because of that.
I don't think signing certs are easy to come by nowadays, but yeah, an "if possible" option should probably be more in along the line of "warn, and let me send unencrypted then", which would support moving to the UI proposed in this bug - as the radio group only makes sense if there is indeed a three state option.
Assignee | ||
Comment 7•15 years ago
|
||
Fix the problem dveditz noted in comment 5.
Attachment #401700 -
Attachment is obsolete: true
Attachment #401722 -
Flags: ui-review?(clarkbw)
Attachment #401722 -
Flags: review?(philringnalda)
Attachment #401700 -
Flags: ui-review?(clarkbw)
Attachment #401700 -
Flags: review?(philringnalda)
Assignee | ||
Updated•15 years ago
|
Target Milestone: --- → Thunderbird 3.0rc1
Comment 8•15 years ago
|
||
Comment on attachment 401722 [details] [diff] [review]
proposed fix, v2
Looks fine to me, as long as Bryan's okay with moving the menuitems up out of the "Ignore This Stuff" submenu they're in.
Probably be a good idea to post to m.d.l10n about the possible need to change the accesskeys, though I think you're right that it's not a certain enough need to call for changing entity names.
Attachment #401722 -
Flags: review?(philringnalda) → review+
Comment 9•15 years ago
|
||
And whether or not certs are "easy" to get is only a tiny part of the problem: most of the people I send mail to either use webmail (many of them as an explicit choice so they can read it from any device anywhere with a browser) or in several cases still use AOL. "Work to solve _that_ problem" means working to eliminate legacy clients from the face of the earth, or to persuade the people using them that it's their fault that they can read mail from everyone else, but not from me, and working to make certs on implanted chips and readers for those implanted chips universally available. Otherwise, I'm simply not going to persuade my friends that my "yeah, let's go fishing up Drift Creek, meet you at the Safeway parking lot at 8" emails are so sacred and sensitive that they must change clients, and/or only be able to read them while they are at home or by installing a Firefox extension and a personal cert (and possibly Firefox) on any computer they happen to want to use to read their mail. And I'm no more likely to set the misnamed "Required", and then for 9999 out of 10000 emails I send have to click ok in a dialog, then uncheck a menuitem, then click send again, than I am to remember to check the menuitem for that 1 out of 10000, or than they are to start carrying their cert everywhere with them.
Should be an interesting race, to see whether you make that world happen before or after someone gets around to implementing jglick's 2001 spec at http://www-archive.mozilla.org/mailnews/specs/security/ for "when possible" so I will at least encrypt mail in the very few cases when I can :)
Updated•15 years ago
|
Attachment #401722 -
Flags: ui-review?(clarkbw) → ui-review+
Comment 10•15 years ago
|
||
Comment on attachment 401722 [details] [diff] [review]
proposed fix, v2
Looks fine to me. For any user who says "I'm going into the Options menu, be back in 5 mins", you should be prepare to wait longer. This change might actually decrease that time by removing the "Do not encrypt this Message" option as if it were something most people actually opt'd into.
Assignee | ||
Updated•15 years ago
|
Attachment #401722 -
Flags: approval-thunderbird3?
Comment 11•15 years ago
|
||
Thank you, Magnus, for championing this UI fix, and making it happen.
Check marks make lousy radio buttons in a menu, especially when they're
also used as ordinary non-radio buttons in the same menu.
I'd be happy to participate in the perennial debate about the options
for "default" behaviors with respect to encrypting outgoing emails, but
I feel pretty strongly that bugs are wrong venue/forum for that debate.
May I suggest mozilla.dev.security.policy? or mozilla.dev.security?
(Shall we have a debate about the right venue for the debate? :)
Updated•15 years ago
|
Attachment #401722 -
Flags: approval-thunderbird3? → approval-thunderbird3+
Assignee | ||
Comment 12•15 years ago
|
||
changeset: 3863:66a98178ec65
http://hg.mozilla.org/comm-central/rev/66a98178ec65
->FIXED
Posted to m.d.l10n.
mozilla.dev.security works for me if you want to discuss how to improve encryption behavior
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 13•15 years ago
|
||
You kept the former and removed the latter.
I haven't tested this code, but, looking at toggleSignMessage(), it looks like the opposite would be wanted.
(I noticed this while working on bug 537219.)
Attachment #419562 -
Flags: review?(mkmelin+mozilla)
Updated•15 years ago
|
Attachment #419562 -
Flags: review?(philringnalda)
Assignee | ||
Comment 14•15 years ago
|
||
Comment on attachment 419562 [details] [diff] [review]
(Bv1) Remove a setNoEncryptionUI() call, Add a setEncryptionUI() call
[Checkin: Comment 15]
Indeed. r=mkmelin
Attachment #419562 -
Flags: review?(mkmelin+mozilla) → review+
Comment 15•15 years ago
|
||
Comment on attachment 419562 [details] [diff] [review]
(Bv1) Remove a setNoEncryptionUI() call, Add a setEncryptionUI() call
[Checkin: Comment 15]
http://hg.mozilla.org/comm-central/rev/379c4de191c5
Attachment #419562 -
Attachment description: (Bv1) Remove a setNoEncryptionUI() call, Adda a setEncryptionUI() call → (Bv1) Remove a setNoEncryptionUI() call, Add a setEncryptionUI() call
[Checkin: Comment 15]
Attachment #419562 -
Flags: review?(philringnalda)
Comment 16•15 years ago
|
||
Comment on attachment 419562 [details] [diff] [review]
(Bv1) Remove a setNoEncryptionUI() call, Add a setEncryptionUI() call
[Checkin: Comment 15]
"approval-thunderbird3.0.1=?":
No risk, fix wrong call.
Attachment #419562 -
Flags: approval-thunderbird3.0.1?
Comment 17•15 years ago
|
||
Comment on attachment 419562 [details] [diff] [review]
(Bv1) Remove a setNoEncryptionUI() call, Add a setEncryptionUI() call
[Checkin: Comment 15]
This is exactly why follow-on patches are not a good idea. It makes tracking extremely hard (if we take this patch on this bug, QA is either going to think we've fixed this bug again, or we're doing something strange) and it would be unclear as to what was fixed in what version from a glance.
I expect we do want this on branch, however please put this patch on a different bug, with a clear, testable description of what it is actually fixing (I've looked at it for 5 minutes and can't easily work it out yet), and then we'll consider it.
Attachment #419562 -
Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1-
Comment 18•15 years ago
|
||
(In reply to comment #17)
> This is exactly why follow-on patches are not a good idea.
(I guess I prefer bugs related to code than to product versions :-/)
> I expect we do want this on branch, however please put this patch on a
> different bug
You moved it to bug 536404 :-)
Whiteboard: [Bv1 is fixed-thunderbird3.1a1]
You need to log in
before you can comment on or make changes to this bug.
Description
•