<Swing Dev> Request review for 7129742 : Unable to view focus in Non-Editable TextArea
Pavel Porvatov
pavel.porvatov at oracle.com
Thu Mar 15 12:57:27 UTC 2012
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 <http://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 <http://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.porvatov at oracle.com <mailto:pavel.porvatov at 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.Potochkin at oracle.com
>> <mailto:Alexander.Potochkin at 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/>
>> <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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120315/c10606db/attachment.html>
More information about the swing-dev
mailing list