Request for review 7155887: ComboBox does not paint focus correctly on GTK L&F (original) (raw)

Pavel Porvatov pavel.porvatov at oracle.com
Tue Apr 3 11:45:00 UTC 2012

Hi Jonathan,

I have several comments:

  1. containerParent instanceof JComboBox && containerParent != null it is not necessary to check "containerParent != null" here

I don't see here any changes

  1. Could you please explain the following changes?

I applied the patch and observe that focused JComboBox looks strange (see attachments): a. Native focus uses solid line b. Native focused JComboBox paints focus rectangle across whole JComboBox

Regards, Pavel

Hi Pavel,

Here's the updated patch, including proposed solution for interior focus. http://cr.openjdk.java.net/~luchsh/71558873/ In the orginal code as I understood, focus is only paint when !interiorFocus, in which case the background shadow and flatBox will shrink a bit to corperate with the outer focus whose size is same as the original component. My proposed solution is to shirink focus line for interior focus, but keep the same way of !interorFocus. could you pls take a look? On 03/27/2012 10:46 PM, Pavel Porvatov wrote: Hi Jonathan,

What do you think about another solution: can we set component state as SynthConstants.FOCUSED before paintTextBackground is invoked. Another solution is to set state as "focused" for ComboBox renderer like the following: if ("ComboBox.renderer".equals(c.getName())) { for (Component comboBoxParent = c.getParent(); comboBoxParent != null; comboBoxParent = comboBoxParent.getParent()) { if (comboBoxParent instanceof JComboBox){ if(comboBoxParent.hasFocus()){ state |= SynthConstants.FOCUSED; } break; } } } without other changes in GTKPainter.java (actually there is some problem with "interiorFocus", but it could be resolved....) See also my answers below. Hi Pavel,

Thanks for review, here's the new patch and my answers are inlined. http://cr.openjdk.java.net/~luchsh/71558872/ On 03/22/2012 10:24 PM, Pavel Porvatov wrote: Hi Jonathan, Hi Swing-dev,

ComboBox on linux GTK L&F does not works as gtk native applications, when get focused, the apperance of Java ComboBox remains unchanged but native GTK ComboBox control will have a outline to indicate it has got focused. The problem seems similar to bug 6947671 ( http://bugs.sun.com/bugdatabase/viewbug.do?bugid=6947671), except that I did not reproduced the problem on Nimbus L&F, so another bug 7155887 (http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7155887) was created for this issue, And here's the proposed patch to fix this problem, http://cr.openjdk.java.net/~luchsh/7155887/ Could anybody please help to take a look? I have several comments about the patch: 1. "c.getName().equals("ComboBox.renderer")": I think we can get NPE here Yes, I've changed it to "ComboBox.renderer".equals(c.getName())

2. + for (Component comboBoxParent = c.getParent(); comboBoxParent != null; comboBoxParent = comboBoxParent + .getParent()) { + if (comboBoxParent instanceof JComboBox + && comboBoxParent.hasFocus()) { + comboBoxFocused = true; + } + } I'm not sure we should do such deep parent investigation. Why don't you check first parent only? javax.swing.CellRendererPane is inserted between the component and renderer, so if check only the first parent, it will retrieve a CellRendererPane object instead of JComboBox component. In the new patch, I added a break when JComboBox is encounterred so to make the effect similar to the first-parent-only approach. I found out the following code (see com.sun.java.swing.plaf.gtk.GTKPainter#paintLabelBackground): if (c instanceof ListCellRenderer && container != null && container.getParent() instanceof JComboBox ) { ... } I think we should use the same pattern 3. "if (ENGINE.paintCachedImage(g, x, y, w, h, id, state) && !comboBoxFocused)" If you are going to ignore ENGINE.paintCachedImage when comboBoxFocused, then there is no need to invoke it at all yes, in the new patch I've changed the order of these two checks. 4. "if (comboBoxFocused || focusSize > 0)" I'm not sure we should paint focus if focusSize == 0 I think there's no need to paint the focus if focusSize ==0, since the focus width and height arguements passed to JNI method nativepaintfocus() will both be zero. That's what I meant! (may be my phrase was not clear enough) Your condition "if (comboBoxFocused || focusSize > 0)" allows to paint focus even if focusSize == 0... Oh, sorry for my misunderstanding, the previous patch indeed got such a problem, but it may not be in the new patch. Regards, Pavel Thanks and best regards! - Jonathan

-------------- next part -------------- A non-text attachment was scrubbed... Name: ver3.png Type: image/png Size: 6286 bytes Desc: not available URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120403/69838650/ver3.png> -------------- next part -------------- A non-text attachment was scrubbed... Name: native.png Type: image/png Size: 5986 bytes Desc: not available URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120403/69838650/native.png>

More information about the swing-dev mailing list