<Swing Dev> [9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Alexander Potochkin]

Hello Semyon

> Alexander, I just double-checked the jdk9 code. See this
>
>     public synchronized void waitForIdle() {
>         checkNotDispatchThread();
>         SunToolkit.flushPendingEvents();
>         ((SunToolkit) Toolkit.getDefaultToolkit()).realSync();
>     }
>
> it calls both flushPendingEvents() and the realSync().

Ups, you are right.

I looked at the JDK8u repo where realSync() is not used by the awt.Robot
and was under impression that in JDK9 we have the same code.

It is good to know that Robot.waitForIdle() is fixed in JDK9!
So it is definitely fine to use it here.

On the other side - we need to change the test when backporting a fix 
from JDK9 to JDK8/7.

So when the backports are required it may still make sense to use the 
plain realSync()
to keep the backports identical to avoid the additional reviews.

Thanks!
alexp

>
> --Semyon
>>
>> The javadoc for Robot.waitForIdle() says:
>> "Waits until all events currently on the event queue have been 
>> processed."
>>
>> realSync() does use syncNativeQueue()
>> so its javadoc states:
>>
>>      * Forces toolkit to synchronize with the native windowing
>>      * sub-system, flushing all pending work and waiting for all the
>>      * events to be processed.  This method guarantees that after
>>      * return no additional Java events will be generated, unless
>>      * cause by user."
>>
>> Thanks
>> alexp
>>
>>>
>>> --Semyon
>>>>
>>>> So I don't recommend using it at all.
>>>>
>>>> The most comprehensive approach is to use the realSync() method 
>>>> from the SunToolkit
>>>> (as many of the Swing jtregs tests do).
>>>>
>>>> This method is definitely more stable than waitForIdle and it works 
>>>> well for most of the tests.
>>>>
>>>> Thanks
>>>> alexp
>>>>
>>>> On 5/11/2016 18:38, Semyon Sadetsky wrote:
>>>>>
>>>>>
>>>>> On 5/11/2016 5:49 PM, Anton Litvinov wrote:
>>>>>> Hi Semyon,
>>>>>>
>>>>>> Thank you for this information. Current version of the regression 
>>>>>> test using the shell script is tested, cross platform and does 
>>>>>> not contain any code specific to Windows platform. This shell 
>>>>>> script is a copy of many other stable regression tests existing 
>>>>>> in "jdk9/jdk/test" directory and is different from them only in 
>>>>>> its commented test header (jtreg options like: @summary, @author) 
>>>>>> and 7 code lines between "###############  YOUR TEST CODE 
>>>>>> HERE!!!!!!! #############" and "###############  END YOUR TEST 
>>>>>> CODE !!!!! ############".
>>>>> I guess other platforms don't remove trailing spaces in the path.
>>>>>>
>>>>>> What is so bad in usage of the well established approach based on 
>>>>>> shell script?
>>>>> Nothing really bad. It just a bit more complex then it might be.
>>>>>
>>>>> You use Thread.sleep and critical sections in the test. Why not to 
>>>>> use AWT robot's waitForIdle() ?
>>>>>
>>>>> --Semyon
>>>>>>
>>>>>> Thank you,
>>>>>> Anton
>>>>>>
>>>>>> On 5/11/2016 5:29 PM, Semyon Sadetsky wrote:
>>>>>>> Hi Anton,
>>>>>>>
>>>>>>> In windows you may use "\\?\" prefix with absolute path of a new 
>>>>>>> folder.
>>>>>>> For example:
>>>>>>> new File("\\\\?\\C:\\tmp\\test2 ").mkdir();
>>>>>>>
>>>>>>> --Semyon
>>>>>>>
>>>>>>> On 5/11/2016 4:47 PM, Anton Litvinov wrote:
>>>>>>>> Hello Sergey,
>>>>>>>>
>>>>>>>> Thank you for review of this fix. No, unfortunately, on MS 
>>>>>>>> Windows OS, if the method "java.io.File.mkdir()" is called on 
>>>>>>>> "java.io.File" instance which contains trailing space 
>>>>>>>> characters in the directory name, the corresponding directory 
>>>>>>>> is created but without trailing space characters in its name in 
>>>>>>>> file system.
>>>>>>>>
>>>>>>>> The method "java.nio.file.Files.createDirectory(Path dir, 
>>>>>>>> FileAttribute<?>... attrs)" cannot be used for this purpose 
>>>>>>>> also, because "java.nio.file.Path" cannot be constructed for 
>>>>>>>> the directory name ending with spaces and 
>>>>>>>> "java.io.File.toPath()" throws the exception 
>>>>>>>> "java.nio.file.InvalidPathException: Trailing char < > at index 
>>>>>>>> N: <DIRECTORY_PATH>".
>>>>>>>>
>>>>>>>> It is possible to create a directory with such a name from the 
>>>>>>>> shell script on MS Windows OS, therefore I decided to use the 
>>>>>>>> shell script for this regression test.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Anton
>>>>>>>>
>>>>>>>> On 5/11/2016 4:16 PM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Anton.
>>>>>>>>> Probably the test can create the folder w/o the shell script?
>>>>>>>>>
>>>>>>>>> On 11.05.16 15:14, Anton Litvinov wrote:
>>>>>>>>>> The bug consists in the fact that the method
>>>>>>>>>> "JFileChooser.getSelectedFile()" returns "java.io.File" 
>>>>>>>>>> object which
>>>>>>>>>> does not contain trailing spaces in the directory name, in 
>>>>>>>>>> spite of the
>>>>>>>>>> fact that the corresponding directory in the file system has 
>>>>>>>>>> trailing
>>>>>>>>>> spaces in its name. The fix deletes the code in the method
>>>>>>>>>> "javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed" 
>>>>>>>>>>
>>>>>>>>>> which deliberately modifies the selected directory string 
>>>>>>>>>> name by
>>>>>>>>>> removing trailing spaces from it.
>>>>>>>>>>
>>>>>>>>>> All automatic regression tests from open and closed sets 
>>>>>>>>>> located in
>>>>>>>>>> "javax/swing/JFileChooser" directories were run on MS Windows 
>>>>>>>>>> 7 OS
>>>>>>>>>> during verification of the fix.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>