Hi Pavel,
Thanks for review, here's the new patch and my answers are inlined.
http://cr.openjdk.java.net/~luchsh/7155887_2/
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/view_bug.do?bug_id=6947671),
except that I did not reproduced the problem on Nimbus L&F, so
another bug
7155887 (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=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.
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
native_paint_focus() will both be zero.
Regards, Pavel
Regards
- Jonathan
