[rfc][icedtea-web][policyeditor] Modal PolicyEditor
Jiri Vanek
jvanek at redhat.com
Tue Mar 25 19:07:27 UTC 2014
On 03/25/2014 03:06 PM, Andrew Azores wrote:
> On 03/25/2014 05:47 AM, Jiri Vanek wrote:
>> Nearly perfect ;)
>>
>> On 03/24/2014 09:39 PM, Andrew Azores wrote:
>>> On 03/24/2014 12:22 PM, Jiri Vanek wrote:
>> ...
>>>>
>>>> jsut noted:
>>>> * Comments will *not* be preserved when PolicyEditor next saves to the
>>>>
>>>> Is it really so hard to store the comments for future save?
>>>
>>> PolicyEditor doesn't even properly parse policy files sometimes, depending on the way the comments are written :) once that's sorted out then being able to store the comments in their own model as well should not be too hard.
>>>
>>
>> Sure. it was jsut undelrine hint
>>
>> <snip>
>>> - }
>>> - });
>>> + public static class PolicyEditorFrame extends JFrame {
>>> + public final PolicyEditor editor;
>>> +
>>> + public PolicyEditorFrame(final PolicyEditor editor) {
>>
>> I doubt this will be ever used - please do it private.
>
> Private, but return the specialized PolicyEditorFrame type anyway from the public getters? Seems a bit odd. I'm not just returning JFrame/JDialog because then there's no easy way to get the actual PolicyEditor instance out of it, to do things like programmatically add codebases. I did have to give the PolicyEditor a reference to its parent frame/dialog in this revision, but I'd rather not use this as a way to hold the reference to the frame/dialog and have eg the security dialogs access the frame/dialog through the editor's reference. That just seems upside down and backward.
I must insists. Please do both "do the actuall work" constructors (public PolicyEditorFrame(final PolicyEditor editor) and public PolicyEditorDialog(final PolicyEditor editor) )private.
>
>>> + super();
>>> + this.editor = editor;
>>> + add(editor);
>>> + this.pack();
>>> + editor.setVisible(true);
>>> + this.setJMenuBar(createMenuBar(this, editor));
>>> +
>>> + setTitle(R("PETitle"));
>>> +
>>> + setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
>>> +
>>> + addWindowListener(new WindowAdapter() {
>>> + @Override
>>> + public void windowClosing(final WindowEvent e) {
>>> + editor.quit();
>>> + dispose();
>>> + }
>>> + });
>>> +
>>> + editor.closeButton.addActionListener(new ActionListener() {
>>> + @Override
>>> + public void actionPerformed(final ActionEvent e) {
>>> + dispose();
>>> + }
>>> + });
>>> + }
>>> + }
>>> +
>>> + public static PolicyEditorFrame getPolicyEditorFrame(final String filepath) {
>>> + return new PolicyEditorFrame(new PolicyEditor(filepath));
>>> + }
>>> +
>>
>> Tehre is really a lot of shared code in this two "methods".
>>
>> PolicyEditorFrame and PolicyEditorDialog.
>>
>> They both have general ancestor -Window. All the operations behind super() call may be done in generalised ancestor.
>
> Window doesn't have setTitle, or setJMenuBar, or setDefaultCloseOperation...
hmm.. I was found with patns down it seams.... The java.awt.Window really do not have it?? Strange...
Anyway - there is really to much duplicate code. Please
nvm - the duplicate code is really awwefull:
+ this.editor = editor;
private artificial forefather?
+ add(editor);
+ this.pack();
window
editor.setVisible(true);
+
+ setTitle(R("PETitle"));
+
this is stupid :)
+ setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
+
this too:-/
Maybe add interfcae for this? :D feel free to elaborate as you wish.
+ addWindowListener(new WindowAdapter() {
+ @Override
+ public void windowClosing(final WindowEvent e) {
+ editor.quit();
+ dispose();
+ }
+ });
+
+ editor.closeButton.addActionListener(new ActionListener() {
+ @Override
+ public void actionPerformed(final ActionEvent e) {
+ dispose();
+ }
+ });
thso eshould be in Window or comon artifical forefather too.
>
>>
>>> + public static class PolicyEditorDialog extends JDialog {
>>> + public final PolicyEditor editor;
>>> +
>>> + public PolicyEditorDialog(final PolicyEditor editor)
>> I doubt this will be ever used - please do it private.
>> {
>>> + super();
>>> + this.editor = editor;
>>> + add(editor);
>>> + this.pack();
>>> + this.setJMenuBar(createMenuBar(this, editor));
>>> +
>>> + editor.setVisible(true);
>>> +
>>> + setTitle(R("PETitle"));
>>> +
>>> + setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
>>> +
>>> + addWindowListener(new WindowAdapter() {
>>> + @Override
>>> + public void windowClosing(final WindowEvent e) {
>>> + editor.quit();
>>> + dispose();
>>> + }
>>> + });
>>> +
>>> + editor.closeButton.addActionListener(new ActionListener() {
>>> + @Override
>>> + public void actionPerformed(final ActionEvent e) {
>>> + dispose();
>>> + }
>>> + });
>>> + }
>>> + }
>>> +
>>> + public static PolicyEditorDialog getPolicyEditorDialog(final String filepath) {
>>> + return new PolicyEditorDialog(new PolicyEditor(filepath));
>>> }
>>>
>>> private void setClosed() {
>>> @@ -370,9 +435,8 @@ public class PolicyEditor extends JFrame
>>> */
>>> private void setAccelerator(final int trigger, final int modifiers, final Action action, final String identifier) {
>>> final KeyStroke key = KeyStroke.getKeyStroke(trigger, modifiers);
>>> - final JRootPane root = getRootPane();
>>> - root.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW).put(key, identifier);
>>> - root.getActionMap().put(identifier, action);
>>> + this.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW).put(key, identifier);
>>> + this.getActionMap().put(identifier, action);
>>> }
>>>
>>> /**
>>> @@ -454,7 +518,6 @@ public class PolicyEditor extends JFrame
>>> }
>>> weakThis.clear();
>>> setClosed();
>>> - dispose();
>>> }
>>>
>>> /**
>>> @@ -649,7 +712,7 @@ public class PolicyEditor extends JFrame
>>> * @param component the component for which to set a mnemonic
>>> * @param mnemonic the mnemonic to set
>>> */
>>> - private void setComponentMnemonic(final AbstractButton component, final String mnemonic) {
>>> + private static void setComponentMnemonic(final AbstractButton component, final String mnemonic) {
>>> final int trig;
>>> try {
>>> trig = Integer.parseInt(mnemonic);
>>> @@ -660,45 +723,61 @@ public class PolicyEditor extends JFrame
>>> component.setMnemonic(trig);
>>> }
>>>
>>> - /**
>>> - * Lay out all controls, tooltips, etc.
>>> - */
>>> - private void setupLayout() {
>>> + private static JMenuBar createMenuBar(final Window window, final PolicyEditor editor) {
>>> + final JMenuBar menuBar = new JMenuBar();
>>> +
>>> final JMenu fileMenu = new JMenu(R("PEFileMenu"));
>>> setComponentMnemonic(fileMenu, R("PEFileMenuMnemonic"));
>>> +
>>> final JMenuItem openItem = new JMenuItem(R("PEOpenMenuItem"));
>>> setComponentMnemonic(openItem, R("PEOpenMenuItemMnemonic"));
>>> openItem.setAccelerator(KeyStroke.getKeyStroke(openItem.getMnemonic(), ActionEvent.CTRL_MASK));
>>> - openItem.addActionListener(openButtonAction);
>>> + openItem.addActionListener(editor.openButtonAction);
>>> fileMenu.add(openItem);
>>> +
>>> final JMenuItem saveItem = new JMenuItem(R("PESaveMenuItem"));
>>> setComponentMnemonic(saveItem, R("PESaveMenuItemMnemonic"));
>>> saveItem.setAccelerator(KeyStroke.getKeyStroke(saveItem.getMnemonic(), ActionEvent.CTRL_MASK));
>>> - saveItem.addActionListener(okButtonAction);
>>> + saveItem.addActionListener(editor.okButtonAction);
>>> fileMenu.add(saveItem);
>>> +
>>> final JMenuItem saveAsItem = new JMenuItem(R("PESaveAsMenuItem"));
>>> setComponentMnemonic(saveAsItem, R("PESaveAsMenuItemMnemonic"));
>>> saveAsItem.setAccelerator(KeyStroke.getKeyStroke(saveAsItem.getMnemonic(), ActionEvent.CTRL_MASK));
>>> - saveAsItem.addActionListener(saveAsButtonAction);
>>> + saveAsItem.addActionListener(editor.saveAsButtonAction);
>>> fileMenu.add(saveAsItem);
>>> +
>>> final JMenuItem exitItem = new JMenuItem(R("PEExitMenuItem"));
>>> setComponentMnemonic(exitItem, R("PEExitMenuItemMnemonic"));
>>> exitItem.setAccelerator(KeyStroke.getKeyStroke(exitItem.getMnemonic(), ActionEvent.CTRL_MASK));
>>> - exitItem.addActionListener(closeButtonAction);
>>> + exitItem.addActionListener(editor.closeButtonAction);
>>> + exitItem.addActionListener(new ActionListener() {
>>> + @Override
>>> + public void actionPerformed(final ActionEvent e) {
>>> + window.dispose();
>>> + }
>>> + });
>>> fileMenu.add(exitItem);
>>> menuBar.add(fileMenu);
>>>
>>> final JMenu viewMenu = new JMenu(R("PEViewMenu"));
>>> setComponentMnemonic(viewMenu, R("PEViewMenuMnemonic"));
>>> +
>>> final JMenuItem customPermissionsItem = new JMenuItem(R("PECustomPermissionsItem"));
>>> setComponentMnemonic(customPermissionsItem, R("PECustomPermissionsItemMnemonic"));
>>> customPermissionsItem.setAccelerator(KeyStroke.getKeyStroke(customPermissionsItem.getMnemonic(), ActionEvent.ALT_MASK));
>>> - customPermissionsItem.addActionListener(viewCustomButtonAction);
>>> + customPermissionsItem.addActionListener(editor.viewCustomButtonAction);
>>>
>>> viewMenu.add(customPermissionsItem);
>>> menuBar.add(viewMenu);
>>> - this.setJMenuBar(menuBar);
>>>
>>> + return menuBar;
>>> + }
>>> +
>>> + /**
>>> + * Lay out all controls, tooltips, etc.
>>> + */
>>> + private void setupLayout() {
>>> final JLabel checkboxLabel = new JLabel();
>>> checkboxLabel.setText(R("PECheckboxLabel"));
>>> checkboxLabel.setBorder(new EmptyBorder(2, 2, 2, 2));
>>> @@ -796,7 +875,6 @@ public class PolicyEditor extends JFrame
>>> add(closeButton, cancelButtonConstraints);
>>>
>>> setMinimumSize(getPreferredSize());
>>> - pack();
>>> }
>>>
>>> /**
>>> @@ -1098,12 +1176,12 @@ public class PolicyEditor extends JFrame
>>> // maybe the user just forgot the -file flag, so try to open anyway
>>> filepath = args[0];
>>> }
>>> - final PolicyEditor editor = new PolicyEditor(filepath);
>>> - editor.setVisible(true);
>>> + final PolicyEditorFrame frame = getPolicyEditorFrame(filepath);
>>> + frame.setVisible(true);
>>> final String codebaseStr = argsMap.get(CODEBASE_FLAG);
>>> if (codebaseStr != null) {
>>> final String[] urls = codebaseStr.split(" ");
>>> - editor.addNewCodebases(urls);
>>> + frame.editor.addNewCodebases(urls);
>>> }
>>> }
>>> });
>>> diff --git a/netx/net/sourceforge/jnlp/util/FileUtils.java b/netx/net/sourceforge/jnlp/util/FileUtils.java
>>> --- a/netx/net/sourceforge/jnlp/util/FileUtils.java
>>> +++ b/netx/net/sourceforge/jnlp/util/FileUtils.java
>>> @@ -18,6 +18,7 @@ package net.sourceforge.jnlp.util;
>>>
>>> import static net.sourceforge.jnlp.runtime.Translator.R;
>>>
>>> +import java.awt.Component;
>>> import java.io.BufferedReader;
>>> import java.io.BufferedWriter;
>>> import java.io.File;
>>> @@ -30,7 +31,6 @@ import java.io.InputStreamReader;
>>> import java.io.OutputStreamWriter;
>>> import java.io.RandomAccessFile;
>>> import java.io.Writer;
>>> -import java.nio.ByteBuffer;
>>> import java.nio.channels.FileChannel;
>>> import java.nio.channels.FileLock;
>>> import java.security.DigestInputStream;
>>> @@ -347,7 +347,7 @@ public final class FileUtils {
>>> * Show a dialog informing the user that the file is currently read-only
>>> * @param frame a {@link JFrame} to act as parent to this dialog
>>> */
>>> - public static void showReadOnlyDialog(final JFrame frame) {
>>> + public static void showReadOnlyDialog(final Component frame) {
>>> SwingUtilities.invokeLater(new Runnable() {
>>> @Override
>>> public void run() {
>>> @@ -361,7 +361,7 @@ public final class FileUtils {
>>> * @param frame a {@link JFrame} to act as parent to this dialog
>>> * @param filePath a {@link String} representing the path to the file we failed to open
>>> */
>>> - public static void showCouldNotOpenFilepathDialog(final JFrame frame, final String filePath) {
>>> + public static void showCouldNotOpenFilepathDialog(final Component frame, final String filePath) {
>> Window will be much more suitable then Component n thsoe calls.
>
> Okay, fixed.
>
>>
>>> showCouldNotOpenDialog(frame, R("RCantOpenFile", filePath));
>>> }
>>>
>>> @@ -371,7 +371,7 @@ public final class FileUtils {
>>> * @param filePath a {@link String} representing the path to the file we failed to open
>>> * @param reason a {@link OpenFileResult} specifying more precisely why we failed to open the file
>>> */
>>> - public static void showCouldNotOpenFileDialog(final JFrame frame, final String filePath, final OpenFileResult reason) {
>>> + public static void showCouldNotOpenFileDialog(final Component frame, final String filePath, final OpenFileResult reason) {
>>> final String message;
>>> switch (reason) {
>>> case CANT_CREATE:
>>> @@ -396,7 +396,7 @@ public final class FileUtils {
>>> * @param filePath a {@link String} representing the path to the file we failed to open
>>> * @param message a {@link String} giving the specific reason the file could not be opened
>>> */
>>> - public static void showCouldNotOpenDialog(final JFrame frame, final String message) {
>>> + public static void showCouldNotOpenDialog(final Component frame, final String message) {
>>> SwingUtilities.invokeLater(new Runnable() {
>>> @Override
>>> public void run() {
>>
>> After thsoe clean up. it will be ready to go.
>>
>> Thanx!
>>
>> J.
>
> And sorry for the noise in this patch as well, but I noticed that somehow it became mixed tabs+spaces, so I just did a :retab along with the actual work.
>
> Also fixed in this patch: Custom Policy Viewer is not frozen due to its parent PolicyEditor's modality.
>
NP.
Otherwise loks ok.
More information about the distro-pkg-dev
mailing list