<AWT Dev> [9] Review request: 8040007 GtkFileDialog strips user inputted filepath

[Alexander Zvegintsev]

Hi Anthony,

thanks for the review, please see the updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8040007/01/

> I assume you've tested this case, and it still works fine, right? 
Sure, it works fine. That is why gtk_file_chooser_get_current_folder() 
is not used and
isFromSameDirectory() was introduced.


--
Thanks,
Alexander.

On 06/09/2014 06:07 PM, Anthony Petrov wrote:
> Hi Alexander,
>
> src/solaris/native/sun/awt/sun_awt_X11_GtkFileDialogPeer.c
>>  176         free(prevDir);
>>  177         prevDir = strdup(dir);
>
> It's unnecessary to re-duplicate the prevDir on each loop iteration 
> here. I suggest to initialize it once instead. The less pointer 
> operations, the less room for bugs.
>
>
>>  229 (*env)->ExceptionClear(env);
>>  230         JNU_ThrowInternalError(env, "Could not instantiate 
>> current folder");
>
> This error message sounds misleading because the code above doesn't 
> instantiate a "folder", it tries to instantiate a string. Also, if 
> exceptions did occur, I believe it's fine to report them as they are. 
> Otherwise, if the pointer is NULL, then this looks more like an OOM 
> than an internal error to me.
>
>
>>  278         //This is a hack for use with "Recent Folders" in gtk 
>> where each
>>  279         //file could have its own directory.
>
> I assume you've tested this case, and it still works fine, right?
>
> -- 
> best regards,
> Anthony
>
> On 6/6/2014 8:00 PM, Alexander Zvegintsev wrote:
>> Hello,
>>
>> please review the fix
>> http://cr.openjdk.java.net/~azvegint/jdk/9/8040007/00/
>> for the bug
>> https://bugs.openjdk.java.net/browse/JDK-8040007
>>
>> With this fix we don't use current folder from native GtkFileChooser
>> anymore, now we build it by ourselves.
>>
>> [1]
>> https://developer.gnome.org/gtk2/stable/GtkFileChooser.html#gtk-file-chooser-get-filenames 
>>
>>
>> [2]
>> https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-path-get-dirname 
>>
>>
>>