201769 - Security panel is confusing - order of controls should be changed

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[Christian Schmidt]

The security panel for each account currently contains controls like this:

[ x ] Digitally sign messages by default

Use this certificate to digitally
[ name of selected certicate ]   [ button: Select ... ] [ Clear ]


The upper checkbox is used to indicate whether the certificate specified below
should be used by default. When a certificate is chosen or cleared, the checkbox
_above_ is enabled or disabled, respectively.

The encryption setting below in the same panel works in the same way.

This is confusing. The selection of a certificate is the most important thing
and should be places first. The checkbox only makes sense when a certificate is
selected. 

When you install a certificate for the first time, at first sight it looks as if
you should somehow check the checkbox (which is impossible because it is
disabled) in order to select a certificate.

Comment 1

[Christian Schmidt]

Attachment: Patch that moves the controls around (made with Patch Maker)

With this patch, the controls will be moved around like this (and likewise with
the encryption controls):

Use this certificate to digitally
[ name of selected certicate ]	 [ button: Select ... ] [ Clear ]

[ x ] Digitally sign messages by default

Comment 2

[[e-mail]]

Would be nice to confirm some UI feedback.

Comment 3

[[e-mail]]

Comment on attachment 120309 [details] [diff] [review]
Patch that moves the controls around (made with Patch Maker)

>+    <checkbox id="identity.sign_mail" iscontrolcontainer="true" wsm_persist="true" genericattr="true" pref="true"
>+              preftype="bool" prefattribute="value"  
>+              prefstring="mail.identity.%identitykey%.sign_mail"
>+              label="&signMessage.label;"/>
Nit: You probably don't need to copy some old attributes
(iscontrolcontainer, genericattr, pref*)

Comment 4

[Christian Schmidt]

Attachment: Updated patch that addresses comment 3

I have removed the attributes mentioned in comment 3. I don't know what those
were used for, but everything appears to be working like before.

I don't have CVS access, so I need help getting this checked in - thanks.

Comment 5

[Matthias Versen [:Matti]]

You still need the SR= and after that you need a a= (or wait until 1.4 is released)

I'll find someone for the checkin if you got reviews..
(Seth's Reeview queue is currently very long)

Comment 6

[Christian Schmidt]

Matti, I am new to the review process, so please forgive me for asking stupid
questions :-)

Should I just wait for Seth's review, or should I request SR from someone else? 
Does the updated patch (attachment 120449 [details] [diff] [review]) need a new review?

Comment 7

[Matthias Versen [:Matti]]

Serg is the only person (AFAIK) who should SR a Mailnews UI bug.
You must wait (and I know this can suck)

Seth can superreview your updated patch (no need to change the request because
Seth should see your updated pathc)

BTW: 
You can see pending request here : http://bugzilla.mozilla.org/request.cgi
and you should possible  read http://mozilla.org/hacking/  (Code review ....)

Comment 8

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 120449 [details] [diff] [review]
Updated patch that addresses comment 3

Carrying forward r=neil.

sr=dmose

Matti: this should be sufficient to land this patch.  It seems like a clear
improvement, it's very small, and jglick is very busy with other things these
days.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in am-smime.xul;
/cvsroot/mozilla/mailnews/extensions/smime/resources/content/am-smime.xul,v  <--
 am-smime.xul
new revision: 1.12; previous revision: 1.11
done