Re: [cp-patches] Patch: Text components home and end actions

[Roman Kennke]

Hi Lillian,

I have some comments on your patch that you might want to have a look
at.

       "TextPaneUI", "javax.swing.plaf.basic.BasicTextPaneUI",
       "TextAreaUI", "javax.swing.plaf.basic.BasicTextAreaUI",
       "TextFieldUI", "javax.swing.plaf.basic.BasicTextFieldUI",
-      "TextPaneUI", "javax.swing.plaf.basic.BasicTextPaneUI",
+      "TextUI", "javax.swing.plaf.basic.BasicTextUI",


I see that TextPaneUI is a double, but why do we need TextUI?
BasicTextUI is abstract and every attempt to instantiate would cause
exceptions. Also, there is no component using TextUI.

+      "TextArea.focusInputMap", new UIDefaults.LazyInputMap(new
Object[] {
+         "UP", "caret-up",
+         "DOWN", "caret-down",
+         "PAGE_UP", "page-up",
+         "PAGE_DOWN", "page-down",
+         "ENTER", "insert-break",
+         "TAB", "insert-tab",
+         "LEFT", "caret-backward",
+         "RIGHT", "caret-forward",
+         "BACK_SPACE", "delete-previous",
+         "ctrl X", "cut-to-clipboard",
+         "ctrl C", "copy-to-clipboard",
+         "ctrl V", "paste-from-clipboard",
+         "shift LEFT", "selection-backward",
+         "shift RIGHT", "selection-forward",
+         "HOME", "caret-begin-line",
+         "END", "caret-end-line",
+         "DELETE", "delete-next"
+      }),

Why not put in all the bindings at once. I pasted the bindings (for
JTextField) from JDK1.5 here:
http://pastebin.com/473135

The others are easy to find out using my attache UIDefaultsInspector
class. Simple call it like that:

java UIDefaultsInspector MyBasicLookAndFeel |grep TextField
(if you want to inspect the MetalLookAndFeel, simply leave out the
MyBasicLookAndFeel parameter).

Also, we have a mauve test for BasicLookAndFeel.initComponentDefaults()
that should be updated.

-    SwingUtilities.replaceUIActionMap(textComponent, getActionMap());
+    SwingUtilities.replaceUIActionMap(textComponent,
textComponent.getActionMap());

This is a bad idea. replaceUIActionMap() replaces the top-level
actionMap for a component with another actionMap. What you would get
from this is a component actionMap A referring to itself as a parent,
this calls for problems.

+    InputMap focusInputMap =
textComponent.getInputMap(JComponent.WHEN_FOCUSED);
+    InputMapUIResource parentInputMap = new InputMapUIResource();
+    ActionMap parentActionMap = new ActionMapUIResource();
+    Object keys[] = focusInputMap.allKeys();

I would think that instead of going through all the keys, we should
fetch all the actions of a textComponent and register them by their
names. Like this:

Action[] actions = textComponent.getActions();
for (int i = 0; i < actions.length; i++)
  {
    String name = actions[i].getValue(Action.NAME);
    parentActionMap.put(name, action[i]);
  }


+    for (int i = 0; i < keys.length; i++)
+      {
+        String act = (String) focusInputMap.get((KeyStroke) keys[i]);
+        Action[] actions = textComponent.getActions();
+        for (int j = 0; j < actions.length; j++)
+          {
+            Action currAction = actions[j];
+            if (currAction != null
+                && (currAction.getValue(Action.NAME).equals(act)))
+              parentActionMap.put(act, new
ActionListenerProxy(currAction, act));
+          }
+        
+        parentInputMap.put(KeyStroke.getKeyStroke(((KeyStroke)
keys[i]).getKeyCode(),
+
convertModifiers(((KeyStroke) keys[i]).getModifiers())),
+                           act);
+        parentInputMap.put(KeyStroke.getKeyStroke(((KeyStroke)
keys[i]).getKeyCode(),
+                                                  ((KeyStroke)
keys[i]).getModifiers()),
+                           act);
+      }

Replace this with the one loop like pasted above. I suppose you are
doing all this work to load the inputMap from the UI Object[]? Then you
might want to use LookAndFeel.loadKeyBindings(). That takes an Object[]
and loads the key bindings into an InputMap.

parentInputMap.setParent(textComponent.getInputMap(JComponent.WHEN_FOCUSED).getParent());
+
parentActionMap.setParent(textComponent.getActionMap().getParent());
+
textComponent.getInputMap(JComponent.WHEN_FOCUSED).setParent(parentInputMap);
+    textComponent.getActionMap().setParent(parentActionMap);

You don't really want to do this. Instead, simply call
SwingUtilities.replaceUIInputMap() or
SwingUtilities.replaceUIActionMap() to properly install the top-level
Input/ActionMaps.

+  private static class ActionListenerProxy extends AbstractAction

This class should not be needed.

+  private int convertModifiers(int mod)

This should also not be needed. Look at the
LookAndFeel.loadKeyBindings() comment above.
 


I hope this helps,
Cheers, Roman

signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil