Community
Participate
Working Groups
I20080415-1646 Please schedule a UI Walkthrough at http://wiki.eclipse.org/UIBPWG_UI_Walkthrough and review the UIs of the Security preference pages, the new security dialogs, and the workflows. There's just too much that needs to be polished in this area, so I won't open individual bugs until the most obvious flaws have been addressed. Security is an important issue, and we cannot get users to trust us if the UI does not look dependable.
We have a couple for the 2 preference pages - 'Services and Providers' and 'Trusted Bundles'. Do you also want to walk through the new Secure Storage work? I've seen some progress in various bugs related to that area already.
I'd be willing to help: We can do the walkthrough together over Sametime, just ping me.
Markus, can you provide a list of things you'd like changed? If possible, I would prefer the list to be specific (i.e., "change ABC to BCD" rather then "ABC is no good"); patches would be even better as many UI tweaks take a lot of time to figure out.
Created attachment 96632 [details] Patch I This patch fixes resizing of the password recovery dialog and changes its layout. The patch eliminates empty space between title ribbon and the dialog area in the login dialog and the password recovery dialog. The patch changes layout of the login dialog and adds a title. Also changes some labels to fit into UI standards.
Patch I applied to CVS Head; keeping bug open for further enhancements.
Created attachment 96634 [details] Patch II The patch uses the same fix to eliminate space between title area and the dialog area in the following dialogs: ExportDialog NewNodeDialog NewValueDialog PasswordRecoveryDialog
Patch II applied to CVS Head; keeping bug open for further enhancements.
Hi Oleg, Me and Markus made a pass over all dialogs and made a list. Your fixes in comment 4 and comment 6 might fix some of the issues, I wasn't able to check that. Comments with screenshots are following...
Created attachment 96785 [details] Security preference page 01. sentences need dot at the end 02. order should correspond to the order on the left 03. wouldn't it be better to put 'Security' below 'General'? We have to be careful with new top level nodes.
Created attachment 96787 [details] Provider and services pref page 10. combo seem misaligned vertically (label should be centered) 11. all margins seem to be wrong, labels are too close to each other (just use the default margins) 12. it's strange to have the numbers in the combo ('0: SUN', 1:SunRsaSign): This should be English human readable names, not the internal names (what does SunRsaSign mean to the user?) 13. Version: margins are wrong, value ('1.6') is too close to 'Version' 14. Description is too long, label has to wrap and go over multiple lines. 15. Use groups only if there is more than one widget in it and if there than one group. Here it would be good enough to just have a label 'Available services:'. 16. The strings in the table should be human readable and English: So rather 'Algorithm Parameter Generator' instead of 'AlgorithmParameterGenerator' 17. Aliases:X509 needs a space after the : 18. How interesting is this table to the user? Is there anything that can be configured here? Maybe this should be hidden in an advanced dialog or not shown at all.
Created attachment 96788 [details] Secure Storage pref page 21. Buttons are too small (see how org.eclipse.jface.dialogs.Dialog.setButtonLayoutData(Button) computes the button size). 22. Top of the buttons should align with table 23. Too much margins on the left and right of the button (compare with JDT preferences pages, like 'organize imports') 24. Pressing 'Logout' seems to have no effect. Maybe a dialog could come up and say what just happened. Or have a label on the page that shows the current connection state. 25. 'Change' needs to be 'Change...' as it shows a input dialog 26. 'Recover' probably also has to be 'Recover...' as I assume it shows a dialog (not sure, I wasn't able to get it enabled and use it) 27. What exactly is this page showing. Is it 'Password' or rather 'Passwords'? A description text would be helpful. I also have no idea what the priorities and what enabled means. 28. 'org.eclipse.equinox.security.windowspassword' is technical. Is there a user readable description instead?
Created attachment 96790 [details] Change password wizard, page 1 31. Title should use title capitalization ('Change Password') 32. OLD should not be capitalized 33. 'OLD password: the values will be decrypted first'. What does that mean? 34. I can't enter a password here. Why showing this page? I would expect to get some password text input fiedls here.
Created attachment 96791 [details] Change password wizard, page 2 41. same comments as for 'Change password wizard, page 1' 42. why is 'Back' disabled?
Created attachment 96792 [details] Password prompt dialog 51. The dialog title should be 'Change Password' like before. 52. What is 'password recovery'. When I first saw this dialog I was clueless, but I had to make a decision. More description would be helpful. Especially explaining if I can later still decide to add password recovery. 53. I would suggest to integrate the password recovery to the wizard (as a 3rd page) (or have all 3 pages in one dialog)
Created attachment 96793 [details] Password recovery dialog 61. wizard title is missing 62. use single quoted: 'Recover...' 63. 'Secure Storage preference page' (not pages) 64. The information shown in the wizard page description is good, but should better be at the bottom of the page. The description should be used to summarize the purpose of the page.
Created attachment 96794 [details] Secure Storage pref page, advanced 71. The combo should contain English, human readable sentences 72. What does 'Default secure preferences', 'Delete' mean? A description would be helpful. 73. Delete button has wrong size 74. Don't use a group for a singe control.
Created attachment 96795 [details] Trusted bundles pref page 81. At least one radio needs to be selected by default 82. margins between radios are too small 83. As ' Allow application code from....' is a sub-option of 'Allow only signed.', give it a extra indent to the right
Created attachment 96797 [details] Secure Storage view 91. the labels are very technical again 92. It looks more like something that belongs in the preferences. Why was is designed as view? 93. What does [Root] mean? Why is it in brackets? Can it be left away? 94. The margin between tree and table is too big (no margin, see type hierarchy as example) 95. Again there are cryptic labels like 'org.eclipse.core.net.proxy.auth'. 96. Why do the CVS URLs contain \2f?
Created attachment 96799 [details] Secure Storage view context menu 101. Why is 'New' only applicable to the root and CVS node? If 'New' isn't applicable to a node, it should not show up in the context menu. If it's applicable the node, but not in the current circumstances, it can be shown disabled. 102. I would have 'New' first, then Delete, then Refresh. 103. 'Delete' should better prompt before really deleting 104. 'Delete' should also work with Delete keyboard shortcut
Created attachment 96800 [details] New node dialog 111. Window title should be 'New XY' 112. Dialog title is missing 113. Description needs a dot at the end 114. What am I creating here? A description would be useful.
Created attachment 96804 [details] Secure Storage view, properties table 121. allow full line selection, so that clicking on the password also selectd the row. 122. ID should better be a human readable, English label ('Login', 'Password')
Created attachment 96805 [details] Show value dialog 131. Use single quotes. 132. Showing my password in clear text. I don't want that! 133. Window title should reflect the action I selected ('Show Value')
Created attachment 96806 [details] Export dialog 141. DECRYPTED should not be in capital letters 142. Browse button need ellipses ('Browse...') 143. Button has wrong width 144. Put the button behind the text field (see for example New Java class wizard) 145. The dialog should verify if the input is valid and only enable ok when this is so
Created attachment 96808 [details] Enter password dialog 151. Dialog title should be something like "Secure Storage Password Configuration" 152. The title message should tell that I have to enter a *new* password and what it is used for, e.g. "Enter a new secure storage password. This is the master password that protects all passwords in the secure storage." 153. Add a read-only, but copyable field that shows the path to the secure storage file, in the IPath#toOSString() format (see e.g. Properties > Resource for a file) 154. Exit button should be called Cancel 155. Login button is not only for login, right? Should be called OK (or Save). 156. The separator line below the title area looks wrongly positioned. See other dialogs (e.g. all the New... dialogs in the SDK) for how to do it. 157. Add a help context ID and make sure that the help button opens a good help page
Created attachment 96810 [details] Enter password dialog 2 Similar problems to dialog above: 161. Dialog title should be something like "Secure Storage Login" 162. Title message is fine here. 163. Add a read-only, but copyable field that shows the path to the secure storage file, in the IPath#toOSString() format (see e.g. Properties > Resource for a file) 164. Exit button should be called Cancel 165. The separator line below the title area looks wrongly positioned. See other dialogs (e.g. all the New... dialogs in the SDK) for how to do it. 166. Add a help context ID and make sure that the help button opens a good help page
I'll be helping Oleg with this list.
Created attachment 97108 [details] Patch for many items listed This patch addresses the following items listed above: Security Preference Page 01 02 03 Trusted Bundles pref page 81 82 83
(In reply to comment #27) Also patch simplifies the option wording on Trusted Bundles pref page to make it clearer at a glance what the options do.
Committed latest patch from Kevin. Updates 'Security' and 'Trusted Bundles' preference pages.
Item 28 addressed in the bug 228918.
#24 addressed in the bug 228943. #22: the "Logout" button applies to all provider, not to the selected one. Hopefully changing its label to "Logout All" helps alleviate the confusion.
Created attachment 97857 [details] Patch Patch changes visible file locations to Path().toOSSting().
Created attachment 98268 [details] Patch III Patch addresses several small issues, in particular from the list above: 62 63 64 152 155 121 27
Patch III applied to CVS Head.
This one will continue into RC1
I hope to provide more patches to address some remaining items
Most of the items in the list have been addressed or became not relevant due to the other changes. If I am not missing anything, only the following items are left: Items left: [22-derived] - make "logout all" button aligned with the text above the table 23. Too much margins on the left and right of the button (compare with JDT preferences pages, like 'organize imports') 94. The margin between tree and table is too big (no margin, see type hierarchy as example) 31 - 34: change password wizard 81. At least one radio needs to be selected by default
Created attachment 98382 [details] Suggested improved layout for the Password tab of the secure storage pref page
(In reply to comment #37) > Most of the items in the list have been addressed or became not relevant due to > the other changes. Thanks for keeping track :) > If I am not missing anything, only the following items are left: > > 81. At least one radio needs to be selected by default Yeah don't know what happened there, it *was* fixed.
Created attachment 98385 [details] Patch for item 81 - At least one radio button needs to be on
(In reply to comment #38) > Created an attachment (id=98382) [details] > Suggested improved layout for the Password tab of the secure storage pref page The description are useful. I would place it at the top. Make it one line that wraps. Would it make sense to call the tab 'Password Providers'? Ar the actual priority values ('2' and '5') of any interest to the user or is the user only interested in the order? If so I would suggest to just sort table and remove the priority column and explain that always the top entry will be used. If always the top entry (the one with highest priority) is taken, are the others still relevant? Can we replace the table with a combo, and the user just selects the provider in use? Or is it more a try them all in order until success? This isn't really clear to me from the description. Do you want the user to be able to change the order? Then 'Up' 'Down' would be useful. I'm still a bit confused about the 'Logout All' button. How is it related to the providers? Does each of the providers have a Logged-In state? Can this be shown in a column (Logged In: Yes, No)? Maybe it would be better to place to button below the table as it acts on all the entries. Is it also possible to 'Log In'? I would change the label 'Change...' to 'Change Password...'. What's not clear to me: Is there a password for each provider? Maybe then it would make sense to have a extra column 'Password Configured: 'Yes' 'No', if you have that information. When I press 'Change...' a wizard opens. It would be good if the wizard would say 'Change password for XY'. 'Recover' would be 'Recover Password...'. The button is sometimes enabled, sometimes not. To understand why, you could add a column: 'Recovery available: 'Yes' 'No'. When I use recovery and successfully enter the questions, I get a message: 'Entries corresponding the password have been successfully unlocked'. What kind of 'Entries' are we talking about here? What 'lock state'? Could this also be shown in the table (tree?)? I think the UI still doesn't help me to understand the structure of information and states.
(In reply to comment #41) The problem is that it is not possible (or desirable) to fit several pages or documentation into a label that would explain everything. The lables and descriptions are constructed in an attempt to achieve balance between describing what actually goes inside the feature and the generally known concepts. Introduction of new concepts is an expensive part and I tried to avoid it as much as possible in the UI. (There will be a separate doc article in Eclipse help on how it works.) On specific points: -> The description are useful. I would place it at the top. Thanks! This is where it originally was placed. It looks better on the bottom. Besides, who was that person that asked to move description in the "Password Recovery" dialog to the bottom? :-) -> 'Change...' to 'Change Password...' -> 'Recover' would be 'Recover Password...' The title of the tab says "Password". Repeating it on every label would seem to be redundant to me and would eat up too much of a screen real estate. -> Priority values Providers are tried in the order of priority, if top one fails, next one is tried, if that one fails next one provider is tried and so on. So, it is not a one value, it is an ordered list. Developers and administrators using the secure storage might be interested in seeing numeric values for priorities. (For instance, you want to create your own provider that fits in between or you'd like to modify extensions so that specific module ends up on top.) -> Do you want the user to be able to change the order? No, not users. -> 'Logout All' button: Maybe it would be better to place the button below the table as it acts on all the entries. It is placed above the table to highlight that it acts on all the entries. -> 'Logout All' button: Does each of the providers have a Logged-In state? Can this be shown in a column (Logged In: Yes, No)? Yes, kind of. This might be added in future, not in 3.4 (What probably muddies the water here is that OS integration modules log in automatically when needed.) -> 'Recover' button: The button is sometimes enabled, sometimes not. To understand why, you could add a column: 'Recovery available: 'Yes' 'No'. The button is available if user provided recovery question/asnwer pairs when the password provider was used. If button is disabled for the password provider, then either user skipped recovery questions (which is fine) or never used this provider. This might be added in future if more people ask about it, but not in 3.4 -> When I use recovery and successfully enter the questions, I get a message: 'Entries corresponding the password have been successfully unlocked'. What kind of 'Entries' are we talking about here? What 'lock state'? Could this also be shown in the table (tree?)? The message could be re-worded. What it means is that data previously encrypted with this password provider can be decrypted. So how about: "Password is sucessfully recovered. Data encrypted with it is going to be accessible in this session."? -------- For the NLS texts please keep in mind long-gone freeze date. At this point changes in NLS text should offer some real benefit to be accepted in for 3.4, and the hard deadline for those will be May 8th.
> -> 'Change...' to 'Change Password...' > -> 'Recover' would be 'Recover Password...' > The title of the tab says "Password". Repeating it on every label would seem to > be redundant to me and would eat up too much of a screen real estate. The title of the table says: 'Providers'. The button acts on an entry in the table. So the user expects that 'Change' changes a provider unless you clarify the button. > -> 'Logout All' button: Maybe it would be better to place the button below the > table as it acts on all the entries. > It is placed above the table to highlight that it acts on all the entries. It looks like one of the buttons on the right. Move it to Top left, over the 'Provider' label. After talking with Kevin it seems that we also could call this 'Clear in-memory password cache'
Created attachment 98458 [details] The screenshot with suggested lables The screenshot shows how lables suggested in the comment 43 going to look like.
(In reply to comment #44) > Created an attachment (id=98458) [details] > The screenshot with suggested lables > > The screenshot shows how lables suggested in the comment 43 going to look like. Maybe "Clear Password Cache" with the implied "in-memory" to keep the button size down. Btw, the button text need to be Book Cap, like [Change Password...]
Created attachment 98597 [details] UI mockups These are some UI mockups with some descriptions I made up (and are probably wrong). The first one is a simple table, the second and third contains an additional 'Details' field to give more information about the selected provider.
(In reply to comment #46) Thank you, I like your design. I am not sure if this is a right time to make those changes so I opened two bugs to keep track of improvoments that would require extension point schema to be modified: Bug 230234 Add "description" text to password providers Bug 230242 Add hints describing capabilities of the password providers (The modifications require addition of a new optional elements to ext. point schema so they will be backward-compatible.) If we were in M6 I'd say yes, let's do it; being in RC1 I am not sure.
Created attachment 99159 [details] Patch IV The patch changes layout to the one proposed in the comment 46 (the "description" is a part of separate enhancement 230234). The patch also fixes original issues 22, 23, 94. Kevin, could you review the patch?
Created attachment 99199 [details] Patch V (slightly modified patch IV) The patch looked pretty good (yahoo Oleg is getting to be a UI hacker! <g>). I updated it with a few minor changes: 1) On the Secure Storage page, there was extra space between the top and the tab folders. This was from an extra unnecessary composite. 2) Save and Delete buttons were being created with a style bit SWT.CENTER. While legal, we generally let the grid data take care of layout and in this case the style had no effect. The password buttons were being created with style SWT.NONE, which is ok since push button is the default, but its nicer to be explicit. So all were changed to SWT.PUSH. 3) I tweaked the text for "Master password providers" to read: "Providers supply 'master' passwords used to encrypt information. The enabled provider with the highest priority is chosen. Check which providers to enable:" This captures the "Check which providers..." part that Martin suggested and I thought was a bit clearer and simpler. 4) The "Delete..." button is ok as just "Delete". We never put ellipsis after Delete even if it prompts for confirmation. I can explain it if you have an hour <g>. You will find other examples in the pref pages of "Remove" with confirmation. On that subject, elsewhere in preferences we use "Remove", not "Delete". But in this case I thought "Delete" carried the right scary semantics so I left it as is.
Thank you! Looks good, the only thing I would prefer to do in a different way is the text from (2): "Providers supply 'master' passwords used to encrypt information. The enabled provider with the highest priority is chosen. Check which providers to enable:" To me, this text implies "go ahead and play with those checkboxes". The meaning I'd like to convey is to "don't touch those checkboxes unless you have a reason to" :-). So I'd say something like: "Providers supply 'master' passwords used to encrypt information. The enabled provider with the highest priority is chosen. A provider can be disabled by un-checking it from this list." How does this sound?
+1 that's even better!
Patch V with message text from comment 50 is applied to CVS Head.
+1 on the patch "Patch for item 81 - At least one radio button needs to be on"; applied to CVS Head.
Created attachment 99353 [details] Patch VI The patch addresses the following points for the change password wizard: 31. Title should use title capitalization ('Change Password') 32. OLD should not be capitalized 33. 'OLD password: the values will be decrypted first'. What does that mean? Change password wizard, page 2 41. same comments as for 'Change password wizard, page 1' As for the #42: (why is 'Back' disabled?): "Back" button has no meaning in that situation, so it is disabled. As for the #34. (I can't enter a password here. Why showing this page? I would expect to get some password text input fiedls here.): the password input is done by providers. In order to making providers simple they don't have separate password change entry point. The need for it will be lessened somewhat by fix for the bug 230242. This is likley to further improved post-3.4.
(In reply to comment #54) > Created an attachment (id=99353) [details] > Patch VI +1 Note that in messages.properties: passwordRecoveryTitleMsg = Please enter Question/Answer pairs. should be passwordRecoveryTitleMsg = Please enter Question/Answer pairs (ie. no period at end of dialog title)
Q: Was there banner art for the password wizard?
Patch VI applied to CVS Head (with title modified with no dot). As for the banner - there is no banner for the password change wizard at this time. I think with this all original concerns specified in this bug are addressed. The following bugs still need to be looked at: Bug 230234 Add "description" text to password providers Bug 230242 Add hints describing capabilities of the password providers Bug 231227 Password recovery question dialog text layout might be improved Great work everybody, time to close this monster.
(In reply to comment #57) > I think with this all original concerns specified in this bug are addressed. > Great work everybody, time to close this monster. Yeah!!! I know we have a keyword "goodbug". This one should be "epicbug"! Thanks all for the hard work.
