<Swing Dev> Review request for 7154030: java.awt.Component.hide() does not repaint parent container
Pavel Porvatov
pavel.porvatov at oracle.com
Mon Mar 26 13:38:27 UTC 2012
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/20120326/74f8c4bb/attachment.html>
More information about the swing-dev
mailing list