<Swing Dev> Review request for 7154030: java.awt.Component.hide() does not repaint parent container
Jonathan Lu
luchsh at linux.vnet.ibm.com
Tue Mar 27 07:46:44 UTC 2012
Hello Pavel,
Here's the updated patch and automatic test for bug 7154030, could you
please take another look?
http://cr.openjdk.java.net/~luchsh/7154030_2/
Thanks and best regards
- Jonathan Lu
On 03/26/2012 09:38 PM, Pavel Porvatov wrote:
> Hi Jonathan,
>> Hi Pavel,
>>
>> Thanks for your explanation.
>>
>> But this bug affects almost all Swing components, hide()'s presence
>> also helps to maintain backward compatibility, so is it possible to
>> put a fix in JComponent to help all the potential affected
>> applications to work correctly?
> Of course that's possible. Do you have final version of the fix?
> Please don't forget write an automatic test.
>> if not, is it there any sunset plan for these deprecated APIs?
> I don't now such plans.
>
> Regards, Pavel
>
> P.S. I removed <AWT Dev>, it seems only Swing will be affected in this fix
>> Thanks and best regards!
>>
>> - Jonathan
>>
>> 2012/3/20 Pavel Porvatov <pavel.porvatov at oracle.com
>> <mailto:pavel.porvatov at oracle.com>>
>>
>> Hi Jonathan,
>>> Hi Artem,
>>>
>>> 2012/3/20 Artem Ananiev <artem.ananiev at oracle.com
>>> <mailto:artem.ananiev at oracle.com>>
>>>
>>> Hi, Jonathan,
>>>
>>> I'm adding swing-dev to CC as we now consider changing Swing
>>> code.
>>>
>>> What you propose sounds technically reasonable, but I don't
>>> think it is worth doing anyway as show() and hide() have
>>> been deprecated for years now.
>>>
>>>
>>> Although show() and hide() have been deprecated for years, in my
>>> opinion supporting these APIs will still benefit many
>>> applications and convince users that Java still has got strong
>>> backward compatibility :D. Any ideas from Swing group?
>> I don't see why the words "backward compatibility" are here.
>> There is a bug in deprecated methods "show" and "hide" (I've
>> checked that jdk5 has the same problem), and that's one
>> additional reason to use setVisible(). I agree with Artem that
>> fixing deprecated API is not a high priority task (but we should
>> keep backward compatibility, of course). I also think, that "to
>> leave all as is" is a good decision for the described problem
>>
>> Regards, Pavel
>>
>>>
>>>
>>> Even if we accept the change in JComponent.hide(), we should
>>> then override show() as well (lightweight component may be
>>> non-opaque, so we should repaint from its parent), so there
>>> will be code duplication. This is one more reason to leave
>>> all as is.
>>>
>>>
>>> Yes, I noticed that code duplication too and am trying to make a
>>> more compact patch for this problem.
>>>
>>> This is my personal opinion, I'm not a Swing expert, though.
>>> Let anyone from the Swing group comment.
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>> On 3/20/2012 12:28 PM, Jonathan Lu wrote:
>>>
>>> Hi Artem,
>>>
>>> Thanks for your time.
>>>
>>> 2012/3/19 Artem Ananiev <artem.ananiev at oracle.com
>>> <mailto:artem.ananiev at oracle.com>
>>> <mailto:artem.ananiev at oracle.com
>>> <mailto:artem.ananiev at oracle.com>>>
>>>
>>> Hi, Jonathan,
>>>
>>> given the code in java.awt.Component, your statement
>>> about
>>> difference between hide() and setVisible(false) looks
>>> pretty strange
>>> to me. Indeed, here is the implementation:
>>>
>>>
>>>
>>> public void show(boolean b) {
>>> if (b) {
>>> show();
>>> } else {
>>> hide();
>>> }
>>> }
>>>
>>> and
>>>
>>> public void setVisible(boolean b) {
>>> show(b);
>>> }
>>>
>>> In JComponent the latter method is overridden and
>>> adds exactly what
>>> you propose: parent.repaint(). This addition makes
>>> sense for
>>> lightweight components (e.g. Swing), but heavyweight
>>> AWT components
>>> shouldn't require this: repaint request is sent from
>>> the native system.
>>>
>>>
>>> Yes, lightweight and heavyweight components differ in
>>> painting. The
>>> original test case only works for the conditions of
>>> lightweight
>>> components, with another test case for heavyweight
>>> components, I found
>>> that the problem could not be reproduced on AWT any
>>> more. I think the
>>> change is only applicable for Swing components, so how
>>> about repaint in
>>> JComponent.hide() like this?
>>>
>>> diff -r cdbb33303ea3
>>> src/share/classes/javax/swing/JComponent.java
>>> --- a/src/share/classes/javax/swing/JComponent.java
>>> Wed Mar 14
>>> 13:50:37 2012 -0700 <tel:2012%20-0700> <tel:2012%20-0700>
>>> +++ b/src/share/classes/javax/swing/JComponent.java
>>> Tue Mar 20
>>> 16:24:09 2012 +0800
>>> @@ -5237,6 +5237,16 @@
>>> }
>>> }
>>>
>>> + public void hide() {
>>> + super.hide();
>>> + Container parent = getParent();
>>> + if (parent != null) {
>>> + Rectangle r = getBounds();
>>> + parent.repaint(r.x, r.y, r.width, r.height);
>>> + parent.invalidate();
>>> + }
>>> + }
>>> +
>>> /**
>>> * Returns whether or not the region of the
>>> specified component is
>>> * obscured by a sibling.
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>>
>>> On 3/15/2012 12:24 PM, Jonathan Lu wrote:
>>>
>>> Hi awt-dev,
>>>
>>> java.awt.Component.hide() was declared as
>>> deprecation and
>>> replaced by
>>> setVisible(boolean), but in my tests, it does not
>>> works in the
>>> same way
>>> as setVisible(false). The reason of this failure
>>> is that
>>> java.awt.Component.hide() does not repaint the
>>> special area it
>>> used to
>>> taken of parent container. Although this is
>>> deprecated method,
>>> it may
>>> still valuable for customers due to compatibility
>>> reason. Bug
>>> 7154030
>>> created for this issue.
>>>
>>> Here's a simple test case to demonstrate this
>>> problem.
>>>
>>> /*
>>> * Copyright (c) 2012 Oracle and/or its
>>> affiliates. All rights
>>> reserved.
>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR
>>> THIS FILE HEADER.
>>> *
>>> * This code is free software; you can
>>> redistribute it and/or
>>> modify it
>>> * under the terms of the GNU General Public
>>> License version 2
>>> only, as
>>> * published by the Free Software Foundation.
>>> *
>>> * This code is distributed in the hope that it
>>> will be useful, but
>>> WITHOUT
>>> * ANY WARRANTY; without even the implied
>>> warranty of
>>> MERCHANTABILITY or
>>> * FITNESS FOR A PARTICULAR PURPOSE. See the
>>> GNU General
>>> Public License
>>> * version 2 for more details (a copy is
>>> included in the
>>> LICENSE file that
>>> * accompanied this code).
>>> *
>>> * You should have received a copy of the GNU
>>> General Public
>>> License
>>> version
>>> * 2 along with this work; if not, write to the
>>> Free Software
>>> Foundation,
>>> * Inc., 51 Franklin St, Fifth Floor, Boston, MA
>>> 02110-1301 USA.
>>> *
>>> * Please contact Oracle, 500 Oracle Parkway,
>>> Redwood Shores,
>>> CA 94065 USA
>>> * or visit www.oracle.com
>>> <http://www.oracle.com> <http://www.oracle.com> if you need
>>> additional information or have any
>>> * questions.
>>> */
>>>
>>> /*
>>> * Portions Copyright (c) 2012 IBM Corporation
>>> */
>>>
>>> import javax.swing.*;
>>>
>>> /* @test 1.1 2012/03/15
>>> @bug 7154030
>>> @run main/manual ComponentHideShowTest.html */
>>>
>>> @SuppressWarnings("serial")
>>> public class ComponetHideShowTest extends JFrame {
>>> JInternalFrame internalFrame;
>>> JButton btn;
>>> JDesktopPane desktop;
>>>
>>> ComponetHideShowTest(String name) {
>>> super(name);
>>> desktop = new JDesktopPane();
>>> setContentPane(desktop);
>>>
>>> setSize(600, 400);
>>> setVisible(true);
>>>
>>> internalFrame = new JInternalFrame("Test
>>> Internal Frame");
>>> internalFrame.setSize(100, 100);
>>> internalFrame.setLocation(10, 10);
>>> internalFrame.setVisible(true)__;
>>> desktop.add(internalFrame);
>>>
>>> btn = new JButton("OK");
>>> btn.setSize(100, 50);
>>> btn.setLocation( 300, 300);
>>> btn.setVisible(true);
>>> desktop.add(btn);
>>>
>>>
>>> setDefaultCloseOperation(__JFrame.EXIT_ON_CLOSE);
>>> }
>>>
>>> @SuppressWarnings("__deprecation")
>>> public void runTest() throws Exception {
>>> Object[] options = { "Yes, I saw it",
>>> "No, I did not
>>> see it!" };
>>>
>>> int ret =
>>> JOptionPane.showOptionDialog(__this,
>>> "Do you see the internal window?",
>>> "InternalFrmaeHideTest",
>>> JOptionPane.YES_NO_OPTION,
>>> JOptionPane.QUESTION_MESSAGE, null,
>>> options, options[1]);
>>>
>>> if (ret == 1 || ret ==
>>> JOptionPane.CLOSED_OPTION) {
>>> throw new Exception("Failed to
>>> display internal
>>> window");
>>> }
>>>
>>> internalFrame.hide();
>>> btn.hide();
>>>
>>> ret = JOptionPane.showOptionDialog(__this,
>>> "Do you see the internal window?",
>>> "InternalFrmaeHideTest",
>>> JOptionPane.YES_NO_OPTION,
>>> JOptionPane.QUESTION_MESSAGE, null,
>>> options, options[1]);
>>>
>>> if (ret == 0 || ret ==
>>> JOptionPane.CLOSED_OPTION) {
>>> throw new Exception("Failed to hide
>>> internal window");
>>> }
>>>
>>> internalFrame.show();
>>> btn.show();
>>>
>>> ret = JOptionPane.showOptionDialog(__this,
>>> "Do you see the internal window?",
>>> "InternalFrmaeHideTest",
>>> JOptionPane.YES_NO_OPTION,
>>> JOptionPane.QUESTION_MESSAGE, null,
>>> options, options[1]);
>>>
>>> if (ret == 1 || ret ==
>>> JOptionPane.CLOSED_OPTION) {
>>> throw new Exception("Failed to hide
>>> internal window");
>>> }
>>> }
>>>
>>> public static void main(String[] args)
>>> throws Exception {
>>> ComponetHideShowTest test = null;
>>> test = new
>>> ComponetHideShowTest("__InternalFrameHideTest");
>>> test.runTest();
>>> }
>>> }
>>>
>>> And here's the patch
>>> http://cr.openjdk.java.net/~__littlee/7154030/
>>> <http://cr.openjdk.java.net/%7E__littlee/7154030/>
>>> <http://cr.openjdk.java.net/%7Elittlee/7154030/>
>>>
>>> Can anybody please help to take a look?
>>>
>>> Cheers!
>>> - Jonathan
>>>
>>>
>>> Best regards!
>>> - Jonathan
>>>
>>>
>>> Thanks a lot !
>>>
>>> - Jonathan
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120327/30c9e2cf/attachment.html>
More information about the swing-dev
mailing list