Bug 323476

Comment 1 Dani Megert 2010-08-24 06:52:12 EDT

Rajesh, can you get access to a Linux machine somehow?

It also looks like the very first time a search string is entered and something selected, it resets the selection in the find field to column 0.

Maybe related to bug 323272?

Comment 2 Dani Megert 2010-08-24 07:31:48 EDT

In your latest patch you removed the runEventQueue() call that Markus added in testShiftEnterReversesSearchDirection, why? This fails now on Linux.

The other failing test is testFocusNotChangedWhenButtonMnemonicPressed.

Comment 3 Dani Megert 2010-08-24 07:35:06 EDT

>I've commented them out for now.
Reverted testShiftEnterReversesSearchDirection to the version from Markus. Please try to test your next version of the patch on a Linux box.

Comment 4 Rajesh 2010-08-24 08:34:18 EDT

(In reply to comment #2)
> In your latest patch you removed the runEventQueue() call that Markus added in
> testShiftEnterReversesSearchDirection, why? This fails now on Linux.
> 
> The other failing test is testFocusNotChangedWhenButtonMnemonicPressed.

The runEventQueue call that Markus added in testShiftEnterReversesSearchDirection is still there in the last patch, it just got moved down one line which shouldn't matter. The other changes in that method are required for the Traverse listener in the application to be called, else by calling traverse on the shell will cause the Selection listener in the application to be called, which means Traverse listener will not get tested. Like you said earlier, the issue could be related to bug 323272. I will look around for a Linux box, not sure if I can get one easily.

Comment 5 Dani Megert 2010-08-24 08:56:41 EDT

>The runEventQueue call that Markus added in
>testShiftEnterReversesSearchDirection is still there in the last patch,
Right.

>I will look around for a Linux box, not sure if I can get one easily.
Otherwise you will have to do it on our build machine once you got introduced to it.

Comment 6 Markus Keller 2010-08-24 16:12:23 EDT

(In reply to comment #4)
> The runEventQueue call that Markus added in
> testShiftEnterReversesSearchDirection is still there in the last patch, it just
> got moved down one line which shouldn't matter.

I haven't debugged it, but I think it does matter. FindReplaceDialog#fActiveShell is set from a ShellListener, and the reason for putting
    runEventQueue();
in front of
    Shell shell= ((Shell)fFindReplaceDialog.get("fActiveShell"));
was to make sure the shell has really been activated (which happens asynchronously on GTK, see bug 286244).

Comment 7 Deepak Azad 2010-08-25 04:43:34 EDT

(In reply to comment #5)
> >I will look around for a Linux box, not sure if I can get one easily.
> Otherwise you will have to do it on our build machine once you got introduced
> to it.
I have emailed you the details of the Linux machine I use for testing.

Comment 8 Rajesh 2010-09-01 07:40:55 EDT

Created attachment 177938 [details]
Fix

Dani, you were right, the shell activation event hasn't happened which is why the runEventQueue call needed to be where it was. Changes work on GTK and Windows.

Comment 9 Dani Megert 2010-09-01 09:06:27 EDT

Thanks for the patch.
Committed to HEAD.
Available in builds >= N20100901-2000.

Comment 10 Dani Megert 2010-09-14 07:33:36 EDT

Verified in I20100914-0100.