[rfc][icedtea-web] PolicyEditor NPE-on-save fix

[Andrew Azores]

On 06/03/2014 10:28 AM, Omair Majid wrote:
> * Andrew Azores <aazores at redhat.com> [2014-05-21 09:52]:
>
>> +++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>> @@ -479,6 +479,14 @@ public class PolicyEditor extends JPanel
>>               if (editor.changesMade) {
>>                   final int save = JOptionPane.showConfirmDialog(this, R("PESaveChanges"));
>>                   if (save == JOptionPane.YES_OPTION) {
>> +                    if (editor.file == null) {
>> +                        final int choice = editor.fileChooser.showSaveDialog(this);
>> +                        if (choice == JFileChooser.APPROVE_OPTION) {
>> +                            editor.file = editor.fileChooser.getSelectedFile();
>> +                        } else if (choice == JFileChooser.CANCEL_OPTION) {
>> +                            return;
>> +                        }
>> +                    }
>>                       editor.savePolicyFile();
>>                   } else if (save == JOptionPane.CANCEL_OPTION) {
>>                       return;
>> @@ -544,6 +552,14 @@ public class PolicyEditor extends JPanel
>>               if (editor.changesMade) {
>>                   final int save = JOptionPane.showConfirmDialog(this, R("PESaveChanges"));
>>                   if (save == JOptionPane.YES_OPTION) {
>> +                    if (editor.file == null) {
>> +                        final int choice = editor.fileChooser.showSaveDialog(this);
>> +                        if (choice == JFileChooser.APPROVE_OPTION) {
>> +                            editor.file = editor.fileChooser.getSelectedFile();
>> +                        } else if (choice == JFileChooser.CANCEL_OPTION) {
>> +                            return;
>> +                        }
>> +                    }
>>                       editor.savePolicyFile();
>>                   } else if (save == JOptionPane.CANCEL_OPTION) {
>>                       return;
> Does it make sense to move these duplicated lines of code into a shared
> method?
>
> Thanks,
> Omair
>

It *can* be done, and I've done it in the attached patch, but the type 
safety starts getting a bit questionable since a Window is needed in 
some places and a PolicyEditorWindow in others. Maybe it could be done 
with a bit better type safety by breaking it into several helpers rather 
than one large one though, but at that point it's going to end up being 
duplicated helper calls anyway.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: policyeditor-save-npe-2.patch
Type: text/x-patch
Size: 3187 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140603/4178f806/policyeditor-save-npe-2.patch>