DnD fails with JTextArea and JTextField (original) (raw)

Neil Richards neil.richards at ngmr.net
Tue Sep 27 13:29:41 UTC 2011

Hi Sean, Pavel,

For ease of review, I've uploaded a webrev [1] with a slightly updated form of the current suggested fix and testcase, in which:

  * The checking code is moved into updateSystemSelection, as
    suggested
  * The testcase is in a (hopefully) sensible location
  * The testcase copyright text is as previously agreed
  * The copyright year is corrected (to 2011)

I'll look to track this conversation for any further changes to the suggested fix, and update the webrev as necessary.

Regards, Neil

[1] http://cr.openjdk.java.net/~ngmr/7049024/webrev.01/

On Mon, 2011-09-26 at 14:45 +0400, Pavel Porvatov wrote:

Hi Sean, > Hi Pavel, > > > Thanks for new comments. I was looking at test\javax\swing > \JTextArea\6940863 > and copied the copyright from there, and just put the initialization > part of the swing > code into EDT. I didn't find realSynch in Toolkit class so used > synch. I'll modify > that. > The realSync is a SunToolkit method. > About the first comment, do you mean to put the testcase and the > fix together > in a webrev ? > Yep! It's hard to reconstruct fix from different mails. Don't forget to select an appropriate folder for the test...

Thanks, Pavel > > 2011/9/26 Pavel Porvatov <pavel.porvatov at oracle.com> > Hi Sean, > > Hi Pavel, > > > > > > Thanks for the comments. I modified the testcase > > according to the comments, > > please have a look at it again. > > I tried to run with jtreg and it runs well. > > > Looks much better. Still have several comments: > > 1. Could you please sent complete fix as a webrev, but not > parts of the fix as a single file? > > 2. Date of copyright... > > 3. You are still using Swing components on non-EDT thread. > As I wrote before take a look at the test\javax\swing > \JSlider\6848475\bug6848475.java test... > > 4. Use toolkit.realSync() instead of toolkit.sync(). BTW: as > described in javadoc realSync cannot be invoked on the EDT > thread... > > Regards, Pavel > > P.S. Sorry for my stubborn =) But on some machines not > accurate tests actually fail (e.g. on Solaris). Therefore > later we must fix tests and that's a really boring task... > > > > > > 2011/9/20 Pavel Porvatov <pavel.porvatov at oracle.com> > > Hi Sean, > > > > The have several comments : > > > > 1. Could you please read > > http://openjdk.java.net/jtreg/ > > is it possible run your test via jtreg? > > > > 2. There is no copyright in the begin of test > > > > 3. There are no jtreg tags > > > > 4. All Swing code must be initialized on the EDT > > thread > > > > 5. Keep test minimal as possible, please. It helps > > other people to understand your code.... E.g. > > there is no need to create JButton with listener. > > > > 6. Note that the "frame.setVisible(true)" doesn't > > guarantee that after that line Frame is visible, > > you should use toolkit.realSync() here > > > > 7. No TODO, please > > > > 8. Are you sure your test should pass if > > exceptions occurs (see your catch blocks) > > > > Please take a look at other tests and use them as > > a good examples.... > > > > Regards, Pavel > > > > > > > Hi Pavel, > > > > > > > > > I wrote a test case for the behavior of > > > DefaultCaret. Please take a look, it is > > > attached. > > > > > > > > > > > > > > > 2011/9/15 Pavel Porvatov > > > <pavel.porvatov at oracle.com> > > > Hi Sean, > > > > Hi Pavel, > > > > > > > > > > > > I'm comfortable with moving the > > > > checking > > > > into DefaultCaret#updateSystemSelection method. > > > > About regression test, I'm not > > > > sure how to write, because it contains > > > > user operation. Can you > > > > > > > > give me a similar test so I can write > > > > one for this bug? > > > > > > > Yes, you can find a lot examples in the > > > test/javax/swing directory by word > > > Robot, e.g. test\javax\swing\JSlider > > > \6848475\bug6848475.java. One hint: use > > > reflection ONLY if there are no another > > > ways to write a test... > > > > > > Regards, Pavel > > > > > > > > > > > > > > > > > > > 2011/9/13 Pavel Porvatov > > > > <pavel.porvatov at oracle.com> > > > > Hi Sean, > > > > > Hi Pavel , > > > > > > > > > > > > > > > I'm sorry I didn't make > > > > > update for this bug for a > > > > > long time, and here is some > > > > > recent investigation. The > > > > > scenario is as follows: > > > > > > > > > > > > > > > Suppose we are dragging > > > > > "abcde" over TextField tf, > > > > > which have "hello dragging" > > > > > as > > > > > its content. When we are > > > > > dragging from start to end, > > > > > there is a cursor moving > > > > > from > > > > > "h" to "g", which means the > > > > > place to insert "abcde" if > > > > > we drop it. > > > > > When we dragging "abcde" > > > > > exit tf, there will be a > > > > > dragExit event, the tf needs > > > > > to > > > > > restore its original status > > > > > after we drag out. Eg. if > > > > > its cursor is between "h" > > > > > and > > > > > "e" in "hello", which > > > > > appears like "h|ello", when > > > > > we are dragging over it, it > > > > > may like > > > > > "hello dr|agging", and when > > > > > drag exit, it needs to be > > > > > "h|ello" again. > > > > > So in dragExit handler, it > > > > > calls > > > > > javax.swing.TransferHandler.cleanup(false), which > > > > > means only to restore the > > > > > original state. cleanup > > > > > calls > > > > > javax.swing.text.JTextComponent.setDropLocation to set the cursor to original > > > > > position. > > > > > And setDropLocation calls > > > > > DefaultCaret.setDot > > > > > and DefaultCaret.moveDot > > > > > to set the state. > > > > > The problem is moveDot > > > > > doesn't know this is just to > > > > > restore the original state, > > > > > it treats the invocation as > > > > > an action to select > > > > > something. And it > > > > > calls updateSystemSelection > > > > > which will > > > > > call java.awt.datatransfer.Clipboard.setContent. And the selected content > > > > > is changed from "abcde" to > > > > > the original selected part > > > > > of "hello dragging", then > > > > > the drop operation finds it > > > > > is not the string dragged > > > > > and nothing is dropped. > > > > > > > > > > > > > > > So I made a simple > > > > > patch(attached) . It just > > > > > check if the textField owns > > > > > focus > > > > > before updateSystemSelection, if it is not focused, it does not treat the moveDot as > > > > > a selection action and does > > > > > not > > > > > call Clipboard.setContent. > > > > > This works on Linux, > > > > > however, DefaultCaret is > > > > > shared by Linux and Windows > > > > > while windows doesn't have > > > > > this problem. So I don't > > > > > think this is a correct > > > > > patch, but it brings my > > > > > question. > > > > > > > > > > > > > > > I think it is strange for > > > > > DefaultCaret to use setDot > > > > > and moveDot to restore > > > > > original state, especially > > > > > moveDot will cause > > > > > an updateSystemSelection > > > > > operation, > > > > > which makes moveDot much > > > > > like an action from user > > > > > instead of just restoring > > > > > state. > > > > > > > > > > > > > > > I'm not sure why it works > > > > > well on windows, but I don't > > > > > think it is right to call > > > > > updateSystemSelection or it > > > > > is not right to use setDot > > > > > and moveDot to restore > > > > > the original state. Is there > > > > > any reason for that ? > > > > Thanks for the patch! I > > > > believe you are right and we > > > > shouldn't update system > > > > selection clipboard when the > > > > component doesn't have focus. > > > > I'd like to modify your fix > > > > and move checking into the > > > > DefaultCaret#updateSystemSelection method: > > > > if (this.dot != this.mark && > > > > component != null && > > > > component.hasFocus()) { > > > > > > > > We also must write regression > > > > tests for fixes if possible, > > > > so an automatic test is needed > > > > as well. Could you please > > > > write a test for the fix? > > > > > > > > > > > > > I'm not sure why it works > > > > well on windows, > > > > > > > > That's because Windows doesn't > > > > have system selection > > > > clipboard... > > > > > > > > > > > > > Is there any reason for > > > > that ? > > > > > > > > No, that's a just bug... > > > > > > > > Regards, Pavel > > > > > > > > > > > > > > 2011/6/6 Pavel Porvatov > > > > > <pavel.porvatov at oracle.com> > > > > > Hi Sean, > > > > > > Hi, > > > > > > > > > > > > I reported, but > > > > > > the system doesn't > > > > > > reply me a bug > > > > > > number. It says > > > > > > "will give me > > > > > > email", > > > > > > but I haven't got > > > > > > one yet. Is this > > > > > > the right process, > > > > > > or I might make a > > > > > > problem when > > > > > > reporting? > > > > > > > > > > > I don't know why the > > > > > system didn't report > > > > > bug ID, but your bug > > > > > was filed > > > > > successfully. You > > > > > can find it here: > > > > > http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7049024 > > > > > > > > > > Regards, Pavel > > > > > > > > > > > > > > > > > 2011/5/27 Pavel > > > > > > Porvatov > > > > > > <pavel.porvatov at oracle.com> > > > > > > Hi Sean, > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > I > > > > > > > have a > > > > > > > testcase > > > > > > > related > > > > > > > to DnD > > > > > > > failure > > > > > > > with > > > > > > > JTextArea and JTextField on linux. The > > > > > > > testcase > > > > > > > is as > > > > > > > follows: > > > > > > > > > > > > > > > > > > > > > /* > > > > > > > * DnDTest.java > > > > > > > */ > > > > > > > import > > > > > > > java.awt.Color; > > > > > > > import > > > > > > > java.awt.Component; > > > > > > > import > > > > > > > java.awt.Dimension; > > > > > > > import > > > > > > > java.awt.FlowLayout; > > > > > > > import > > > > > > > java.awt.Frame; > > > > > > > import > > > > > > > java.awt.event.WindowAdapter; > > > > > > > import > > > > > > > java.awt.event.WindowEvent; > > > > > > > > > > > > > > > > > > > > > import > > > > > > > javax.swing.JTextArea; > > > > > > > import > > > > > > > javax.swing.JTextField; > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > public > > > > > > > class > > > > > > > DnDTest > > > > > > > extends > > > > > > > Frame { > > > > > > > Component c; > > > > > > > public > > > > > > > DnDTest() { > > > > > > > super("Single Frame --- AWT Frame"); > > > > > > > super.setBackground(Color.gray); > > > > > > > > > > > > > > // set > > > > > > > layout > > > > > > > here. > > > > > > > setLayout(new FlowLayout()); > > > > > > > > > > > > > > c = new > > > > > > > JTextArea("JTextArea component"); > > > > > > > c.setPreferredSize(new Dimension(400, 100)); > > > > > > > add(c); > > > > > > > > > > > > > > c = new > > > > > > > JTextField("JTextField component(No IM)"); > > > > > > > c.setPreferredSize(new Dimension(400, 20)); > > > > > > > add(c); > > > > > > > > > > > > > > addWindowListener(new WindowAdapter() { > > > > > > > public > > > > > > > void > > > > > > > windowClosing(WindowEvent event) { > > > > > > > System.exit(0); > > > > > > > } > > > > > > > }); > > > > > > > setSize(850, 360); > > > > > > > setVisible(true); > > > > > > > } > > > > > > > > > > > > > > public > > > > > > > static > > > > > > > void > > > > > > > main(String[] args) { > > > > > > > new > > > > > > > DnDTest(); > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Reproduce steps: > > > > > > > 1. Run > > > > > > > the > > > > > > > testcase > > > > > > > with > > > > > > > b143 > > > > > > > 2. Open > > > > > > > a new > > > > > > > file > > > > > > > with > > > > > > > gedit > > > > > > > and > > > > > > > input > > > > > > > some > > > > > > > words > > > > > > > like > > > > > > > "abcde" > > > > > > > 3. Drag > > > > > > > "abcde" > > > > > > > into > > > > > > > JTextField and drop it there. > > > > > > > 4. Once > > > > > > > more, > > > > > > > drag > > > > > > > "abcde" > > > > > > > into > > > > > > > JTextField and then move out of the Frame (keep draging) and drag into JTextField again and drop it. > > > > > > > > > > > > > > > > > > > > > Expectation: > > > > > > > The > > > > > > > second > > > > > > > DnD > > > > > > > inputs > > > > > > > another > > > > > > > "abcde" > > > > > > > into JTextField. > > > > > > > > > > > > > > > > > > > > > Result: > > > > > > > The > > > > > > > second > > > > > > > DnD > > > > > > > inputs > > > > > > > nothing > > > > > > > into JTextField. > > > > > > Yes, looks > > > > > > like a > > > > > > bug. The > > > > > > test case > > > > > > works on > > > > > > Windows as > > > > > > expected. > > > > > > > > > > > > > > > > > > > > > > > > > > > Investigation: > > > > > > > The > > > > > > > JTextArea as well has this problem, and in step 4, if we drag "abcde" over JTextField and then drop into JTextArea, nothing > > > > > > > is input > > > > > > > into > > > > > > > JTextArea either. However, if "abcde" is drag into JTextField or JTextArea directly or when JTextArea/Field are > > > > > > > empty as > > > > > > > in step > > > > > > > 2, it > > > > > > > works. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Are > > > > > > > there > > > > > > > any > > > > > > > comments? And can anyone file a bug for it please ? > > > > > > > > > > > > > Anybody > > > > > > can file a > > > > > > bug, > > > > > > http://bugreport.sun.com/bugreport/ > > > > > > > > > > > > Regards, > > > > > > Pavel > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best Regards, > > > > > > Sean Chou > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best Regards, > > > > > Sean Chou > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best Regards, > > > > Sean Chou > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Best Regards, > > > Sean Chou > > > > > > > > > > > > > > > > > -- > > Best Regards, > > Sean Chou > > > > > > > > > > > -- > Best Regards, > Sean Chou > >

-- Unless stated above: IBM email: neil_richards at uk.ibm.com IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

More information about the swing-dev mailing list