Force JPopup to be always heavyweight (original) (raw)

Pavel Porvatov pavel.porvatov at oracle.com
Sat Apr 21 15:53:14 UTC 2012

Hi Mario,

See comments below...

2012/4/16 Pavel Porvatov<pavel.porvatov at oracle.com>:

Hello Pavel,

About the test, I'm not sure if you need something more specific, but I was thinking to simply test if when isLightWeightPopupEnabled is set we create an heavy or lightweight popup, any other ideas? That's a good way I think Here is the webrev: http://cr.openjdk.java.net/~neugens/6800513/ Note that MEDIUMWEIGHT is still used in some cases (PopupFactory.getPopup): if (owner == null || invokerInHeavyWeightPopup(owner)) { popupType = HEAVYWEIGHTPOPUP; } else if (popupType == LIGHTWEIGHTPOPUP&& !(contents instanceof JToolTip)&& !(contents instanceof JPopupMenu)) { popupType = MEDIUMWEIGHTPOPUP; } Yes, ToolTipManager uses MEDIUM_WEIGHT as well I'm not sure if we want to replace this as well with full HW (I would do it, in fact). Actually we can't use HEAVY_WEIGHT always because there are some tricky usages. E.g. you can create a frame with some transparency. After that HEAVY_WEIGHT popups will be opaque, but LIGHT_WEIGHT will have the same transparency... Also I was thinking to have a very small optimization, if we are using an HW popups, there's no need to do this check: // Check if the parent component is an option pane. If so we need to // force a heavy weight popup in order to have event dispatching work // correctly. Component c = owner; while (c != null) { if (c instanceof JComponent) { if (((JComponent)c).getClientProperty( PopupFactoryFORCEHEAVYWEIGHTPOPUP) == Boolean.TRUE) { popupType = HEAVYWEIGHTPOPUP; break; } } else if (c instanceof Window) { Window w = (Window) c; if (!w.isOpaque() || w.getOpacity()< 1 || w.getShape() != null) { popupType = HEAVYWEIGHTPOPUP; break; } } c = c.getParent(); } We can save a loop and a couple of instanceof checks (although I remember we proved with Jim and Phil that instanceof don't really introduce any speed penalization). What do you think? I didn't catch details of your optimization. Note that we are not going to remove lightweight popups because of the described above transparency example. I see another optimization: because of the PopupFactory#getPopupType only "increases" weight we can replace all "popupType = HEAVY_WEIGHT_POPUP" by "return HEAVY_WEIGHT_POPUP". But I'd preffer don't touch code that doesn't relative to the fix

About the test:

  1. Now is 2012 :)
  2. You must access to Swing components only from the EDT (see clickOnComponent(final Component comp) and other methods)
  3. Don't use long lines please
  4. Could you please use simplest constractions when possible. E.g. a. frame.setMinimumSize(new Dimension(500, 500)); frame.pack();

can be replaced by frame.setSize(...)

b. loop final Map<String, Boolean> tests = new HashMap<>(); tests.put("javax.swing.PopupFactory$HeavyWeightPopup", false); tests.put("javax.swing.PopupFactory$LightWeightPopup", true);

     for (final String test : tests.keySet()) {

can be replaced by two simple invocations

c. NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException can be replaced by Exception

d. robot.delay(50); robot.mousePress(InputEvent.BUTTON1_MASK); robot.delay(50); Just use Robot#setAutoDelay

etc.

  1. latch must be volitile. After test rewriting I think this variable can be removed at all

Note that tests should be readable and simplest as far as possible

Regards, Pavel

More information about the swing-dev mailing list