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

[Lillian Angel]

Hi Roman,

> 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",
> 

Fixed

> 
> 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

I added them all in.

> 
> 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.

I will do this next.

> 
> -    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.

Fixed.

> 
> +    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]);
>   }

Fixed.

> 
> 
> +    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.

I made this loop more efficent by far. I attempted to use the
loadKeyBindings, but I realized its not possible. I need to pass the
entire array of strings from the Text*.focusInputMap. But if I use the
UIManager to get the focusInputMap, an instance of InputMapUIResource is
returned (not an array). This InputMap contains half as many entries as
those in the array of strings (the key/value pairs). To get the values
of the input map, keys() is called and this returns an array of all the
KeyStrokes- not the strings. Therefore, in this case loadKeyBindings is
not helpful.

> 
> 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.

This is setting the parents of the map. I don't see why I should call
replaceUI*Map here.

2005-12-20  Lillian Angel  <address@hidden>

        * javax/swing/UIDefaults.java:
        (LazyInputMap): InputMap should be an InputMapUIResource.
        * javax/swing/plaf/basic/BasicLookAndFeel.java
        (initComponentDefaults): Added all key bindings for Text*.
        * javax/swing/plaf/basic/BasicTextUI.java
        (installKeyboardActions): Fixed call to replaceUIActionMap
        to create a new ActionMap from textComponent's actions. Prevents
        an infinite loop. Fixed loop to set the parentActionMap and the
        parentInputMap.
        (ActionListenerProxy): Removed. No longer needed.
        (convertModifiers): Likewise.

patch.diff
Description: Text Data