[rfc][icedtea-web] Temporary permissions

[Jiri Vanek]

On 03/25/2014 06:34 PM, Andrew Azores wrote:
> On 03/25/2014 05:40 AM, Jiri Vanek wrote:
>> Generally ok, except one important nit:
>>
>> final JMenuItem noCmdExec = new JMenuItem(R("STempPermNoExec"));
>> + noCmdExec.addActionListener(new TemporaryPermissionsListener(
>> + Permissions.READ_LOCAL_FILES, Permissions.WRITE_LOCAL_FILES,
>> + Permissions.READ_TMP_FILES, Permissions.WRITE_TMP_FILES,
>> + Permissions.READ_PROPERTIES, Permissions.WRITE_PROPERTIES,
>> + Permissions.CLIPBOARD_ACCESS, Permissions.PLAY_AUDIO,
>> + Permissions.PRINT_DOCUMENTS, Permissions.JAVA_REFLECTION,
>> + Permissions.NETWORK_ACCESS));
>> + policyMenu.add(noCmdExec);
>>
>> And similar:
>> are - duplicated.
>>
>>
>> You can push, as similar evil is already done.
>>
>> However refactoring, asn asap changeset is needed.
>>
>> the ||| advanced settings button will be encapsulated, so non of its code is duplicated.
>> You can have your own "extends jbutton" hwhich will do all this logic. And you add jsut insntance of this button to the panes.
>>
>> Also all the :
>> new TemporaryPermissionsListener(
>> + Permissions.READ_LOCAL_FILES, Permissions.WRITE_LOCAL_FILES,
>> + Permissions.READ_TMP_FILES, Permissions.WRITE_TMP_FILES,
>> + Permissions.READ_PROPERTIES, Permissions.WRITE_PROPERTIES,
>> + Permissions.CLIPBOARD_ACCESS, Permissions.PLAY_AUDIO,
>> + Permissions.PRINT_DOCUMENTS, Permissions.JAVA_REFLECTION,
>> + Permissions.NETWORK_ACCESS));
>> + policyMenu.add(noCmdExec);
>>
>> should be encapsualted. Those permissionListeners may be stored in some static class as static constantsand just used as singletons. or not?
>> If not then they can be provided by some factory methods. But not spred in gui code like this.
>>
>> Yes, just code celaning, but how it is now, i do not look nicely.
>>
>> Sorry for troubles:(
>> J.
>>
>
> You're right, it was getting very messy, and the code for the button/menu stuff definitely should have been shared.
>
> So now, behold the monster you've asked for... ! :D
>
> 1) Button is pulled out into its own class and used in the two dialogs
> 2) Popup menu is actually even simpler to make appear now (less futzing with position on screen)
> 3) !! PolicyEditorPermissions are reused. It's some slightly scary reflection with a bit of regex sprinkled in, but it works perfect.
> 3b) Some of the temp permissions have been removed because PolicyEditorPermissions don't yet have the right counterparts. But there's another patch I sent earlier that adds all of those permissions as well, so then they can be added in quite quickly here after
> 4) Temp permission types and menu entries are much much cleaner to define
>


hmhmhm - just see:
+STempSoundOnly=Play audio
  it is same issue as in case of reflection. it should probably allow both recording and playing. UTwo permissions under one checkbox.

- maybe as tmp hack can serve some better grouping of checkboxes, with one main - whcih will select/unselect all underlying. By this you will have your one-permission-per-checkbox - and user will not be bored by to much on-first-glance-visible permissions. Maybe I can do thsis tomorrow? I wil enjoy it :)


+    public static Permission getPermission(final PolicyEditorPermissions editorPermission) {

arrgh thats monster..... Why reflection!!?!??! No way!!!  it is our class. Please do something public or whatever...



Otherwise it is not os monsterous :)