Hi Pavel,
As awt team has committed the related bug(
http://hg.openjdk.java.net/jdk8/awt/jdk/rev/96340349e35b), I prepared the
webrev for this
bug(7129742<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7129742>)
again.
The new webrev: http://cr.openjdk.java.net/~zhouyx/7129742/webrev.05/ .
The sunbug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7129742
Previous discussion:
http://mail.openjdk.java.net/pipermail/swing-dev/2012-March/001923.html
Please take a look once more, thank you.
On Thu, Mar 15, 2012 at 8:57 PM, Pavel Porvatov
<pavel.porva...@oracle.com>wrote:
> Hi Sean,
>
>
> Hi Pavel,
>
> Thanks for your comments.
>
> About the 1st question, I modified the testcase a little to demonstrate
> the problem, testcase is pasted at the end.
>
> If textArea.setEditable(true) is executed, the frame disposes, but
> application doesn't exit.
> If textArea.setEditable(false), the application exits as normal.
>
> java 1.7.0_01 and latest java8 code are tested. And TextField contains
> this problem as well. I'll add it to fix later.
> Shall I report a separate bug for it ?
>
> Yes. I think it will be better if you fix the described above problem as a
> separate bug. Note that it should be reviewed by the AWT team...
>
>
>
> About the 2nd question, I was driven by the comment "// TODO : fix this
> duplicate code". Is there any strong reason to keep it ?
>
> I'm absolutely agree with removing duplicate code. My second question was
> about added by you "jtext.getCaret().setVisible(false)" in the
> XTextAreaPeer.java class. I wrote:
> 2. Why don't we need the same code in the XTextFieldPeer class?
>
> As I can see you've noticed that XTextFieldPeer should be fixed as well
> (in a separate fix with XTextAreaPeer)
>
> Regards, Pavel
>
>
>
> /////////////////////////////////// a testcase
> ////////////////////////////////////////////////////////////
> /*
> * 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 if you need additional information or have any
> * questions.
> */
>
> /*
> * Portions Copyright (c) 2012 IBM Corporation
> */
>
> /* @test
> * @bug
> * @summary editable TextArea blocks gui app from dispose.
> * @author Sean Chou
> */
>
> import java.awt.FlowLayout;
> import java.awt.Frame;
> import java.awt.TextArea;
> import java.awt.Toolkit;
> import java.lang.reflect.Field;
>
> import javax.swing.JFrame;
> import javax.swing.JTextArea;
> import javax.swing.SwingUtilities;
> import javax.swing.text.DefaultCaret;
>
> import sun.awt.SunToolkit;
>
> public class TextAreaDisposeBug {
> public static volatile boolean passed = false;
>
> public static Frame frame = null;
> public static TextArea textArea = null;
>
> public static DefaultCaret caret = null;
> public static Class XTextAreaPeerClzz = null;
>
> public static void main(String[] args) throws Exception {
> SunToolkit toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
> SwingUtilities.invokeAndWait(new Runnable() {
> @Override
> public void run() {
> frame = new JFrame("Test");
>
> textArea = new TextArea("editable textArea");
> textArea.setEditable(true);
> // textArea.setEditable(false);
>
> frame.setLayout(new FlowLayout());
> frame.add(textArea);
>
> frame.pack();
> frame.setVisible(true);
>
> }
> });
> toolkit.realSync();
>
> SwingUtilities.invokeAndWait(new Runnable() {
> @Override
> public void run() {
> frame.dispose();
> }
> });
> toolkit.realSync();
> }
>
> }
>
>
>
> //////////////////////////////////////////////////////////////////////////////////////////
> /*
> * 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 if you need additional information or have any
> * questions.
> */
>
> /*
> * Portions Copyright (c) 2012 IBM Corporation
> */
>
>
> /* @test
> * @bug
> * @summary editable TextField blocks gui app from dispose.
> * @author Sean Chou
> */
>
> import java.awt.FlowLayout;
> import java.awt.Frame;
> import java.awt.TextField;
> import java.awt.Toolkit;
> import java.lang.reflect.Field;
>
> import javax.swing.JFrame;
> import javax.swing.JTextArea;
> import javax.swing.SwingUtilities;
> import javax.swing.text.DefaultCaret;
>
> import sun.awt.SunToolkit;
>
> public class TextFieldDisposeBug {
> public static volatile boolean passed = false;
>
> public static Frame frame = null;
> public static TextField textField = null;
>
> public static DefaultCaret caret = null;
> public static Class XTextAreaPeerClzz = null;
>
> public static void main(String[] args) throws Exception {
> SunToolkit toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
> SwingUtilities.invokeAndWait(new Runnable() {
> @Override
> public void run() {
> frame = new JFrame("Test");
>
> textField = new TextField("editable textArea");
> // textField.setEditable(true);
> textField.setEditable(false);
>
> frame.setLayout(new FlowLayout());
> frame.add(textField);
>
> frame.pack();
> frame.setVisible(true);
>
> }
> });
> toolkit.realSync();
>
> SwingUtilities.invokeAndWait(new Runnable() {
> @Override
> public void run() {
> frame.dispose();
> }
> });
> toolkit.realSync();
> }
>
> }
>
>
> On Tue, Mar 13, 2012 at 9:49 PM, Pavel Porvatov <
> pavel.porva...@oracle.com> wrote:
>
>> Hi Sean,
>>
>> I have a couple questions about the following code in the
>> XTextAreaPeer.java class
>> + // visible caret has a timer thread, which must be stopped
>> + jtext.getCaret().setVisible(false);
>>
>> 1. Why this code wasn't needed before your fix, when caret was visible
>> for enabled text area?
>> 2. Why don't we need the same code in the XTextFieldPeer class?
>>
>> About the bug7129742 test:
>> Because of the passed field is used from different threads you must use
>> synchronization or mark the field as a volatile.
>>
>> Regards, Pavel
>>
>>
>> Hi Alexander,
>>
>> XTextFieldPeer and XTextAreaPeer have a same inner
>> class XAWTCaret, and in XTextAreaPeer there is also a comment:
>> "// TODO : fix this duplicate code " before XAWTCaret . So I removed
>> the XAWTCaret in XTextFieldPeer and changed the
>> XAWTCaret into a static class, so XTextFieldPeer can
>> use XAWTCaret from XTextAreaPeer .
>>
>> As XAWTCaret is only used in the following
>> method in both XTextAreaPeer and XTextFieldPeer .
>> protected Caret createCaret() {
>> return new XAWTCaret();
>> }
>> I think this modification should not bring side effect.
>>
>> On Mon, Mar 12, 2012 at 1:27 AM, Alexander Potochkin <
>> alexander.potoch...@oracle.com> wrote:
>>
>>> Hello Sean
>>>
>>> Could you give more details about your changes in XTextFieldPeer?
>>>
>>> Thanks
>>> alexp
>>>
>>> Hi all,
>>>>
>>>> I updated the patch to as suggested and simplified the testcase .
>>>> Would anyone like to take a look again ? Thanks.
>>>>
>>>> The webrev is at :
>>>> http://cr.openjdk.java.net/~zhouyx/7129742/webrev.04/ <
>>>> http://cr.openjdk.java.net/%7Ezhouyx/7129742/webrev.04/>
>>>>
>>>>
>>>> Previous discussion at :
>>>>
>>>> http://mail.openjdk.java.net/pipermail/swing-dev/2012-February/001913.html
>>>>
>>>> --
>>>> Best Regards,
>>>> Sean Chou
>>>>
>>>>
>>>
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>>
>>
>
>
> --
> Best Regards,
> Sean Chou
>
>
>
--
Best Regards,
Sean Chou
