<AWT Dev> [8] Review request for JDK-8029263, , The user's default browser can not launch after we click the button, and there is an IOException shown in the log txt (java.io.IOException)

Fri Dec 13 04:33:11 PST 2013

Hi Alexander,

The fix looks much better. Though I still don't understand why you 
declare the i variable in update_supported_actions() so far from the 
place where it is used. What would go wrong if you moved the declaration 
inside the if() block just above your while() loop? It's not referenced 
from anywhere else, is it?

Thanks for filing a new RFE. Please update its Description with some 
meaningful text that would describe what actually is requested/needs to 
be investigated.

--
best regards,
Anthony

On 12/13/2013 02:16 PM, Alexander Zvegintsev wrote:
> Hi Anthony,
>
> the updated webrev is here:
> http://cr.openjdk.java.net/~azvegint/jdk/8/8029263/webrev.01/
> I also removed a redundant null check in XDesktopPeer.isSupported()
>
> The new RFE for 9 is here:
> https://bugs.openjdk.java.net/browse/JDK-8030100
>
>> A follow-up question regarding #3: do you know which specific
>> library/software module needs to be installed in order to support
>> those other URI schemes? It might be useful to add this info to the
>> bug report so that people know what they need to install additionally
>> if they need to access such URIs.
> Unfortunately I don't know currently. But I think this info can be added
> later, when I will investigate a new RFE.
>
> Thanks,
>
> Alexander.
>
> On 12/13/2013 01:49 AM, Anthony Petrov wrote:
>> Thanks for the clarifications, Alexander. Looking forward to an
>> updated webrev.
>>
>> As for #4, I understand that we want to minimize the risk for this fix
>> now, so late in the release. And I'm fine with that. However, I still
>> suggest to file an RFE targeted for 9 to enable this behavior for
>> Linux as well. I reckon a Linux system can also be configured not to
>> support additional URI schemes, so in general, I see no reason to skip
>> this check on Linux.
>>
>> A follow-up question regarding #3: do you know which specific
>> library/software module needs to be installed in order to support
>> those other URI schemes? It might be useful to add this info to the
>> bug report so that people know what they need to install additionally
>> if they need to access such URIs.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 12/13/2013 01:32 AM, Alexander Zvegintsev wrote:
>>> Hi Anthony,
>>>
>>> Please see inline
>>> 13.12.2013 0:30, Anthony Petrov wrote:
>>>> Hi Alexander,
>>>>
>>>> 1. Please add your evaluation to the bug report.
>>>>
>>>> 2. In gtk2_interface.c: can we declare the i variable somewhere closer
>>>> to where it is used? What's the point with all these forward
>>>> declarations? We don't compile with C89, do we?
>>> Sure.
>>>>
>>>> 3. Checking for http: to enable the BROWSE action looks reasonable to
>>>> me. And I suggest to check for the mailto: scheme separately, so that
>>>> the MAIL action is only enabled if it's supported.
>>>  From my testing if we have http scheme in this list(list of supported
>>> URI schemes) we are able to open https://, ftp:// and mail: URIs (BTW
>>> there is no https:// and mail: schemes in this list, and mail: is not
>>> actually a valid URI).
>>> If we have only file scheme in this list all other URIs will not work
>>> and gtk_show_uri() will return "Operation not supported" message for
>>> such URIs.
>>> So this part of code is just an educated guess to check is gvfs
>>> supported or not. It may be fragile(it depends on environment variables
>>> set) but it is better then nothing.
>>>
>>> Most of machines with Solaris 11 installed which I tested always return
>>> only file scheme in this list for 64-bit, so in general case we will
>>> support only an OPEN action.
>>>
>>>> 4.
>>>>>  534 #ifdef __solaris__
>>>>>  535             update_supported_actions(env);
>>>>>  536 #endif
>>>>
>>>> Have you tested this on Linux with your patch applied? How is the list
>>>> of supported actions supposed to be populated on that platform?
>>> Linux behaves as before the fix:
>>> For Linux this list is already populated in initializer and remains
>>> untouched from native:
>>>> +    private static final List<Action> supportedActions
>>>> +            = new ArrayList<>(Arrays.asList(Action.OPEN,
>>>> Action.MAIL, Action.BROWSE))
>>> The list of supported actions is the same as before the patch (OPEN,
>>> MAIL, BROWSE). I tested this on OEL 5, 6, Ubuntu 13.04 all works as
>>> expected.
>>> So only for Solaris we clear this list and populate it from a native.
>>>
>>>>
>>>> 5. In XDesktopPeer.java: please move the comment at line 51 above the
>>>> field that it describes.
>>>
>>> --
>>> Thanks,
>>> Alexander.
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 12/12/2013 09:03 PM, Alexander Zvegintsev wrote:
>>>>> Hello AWT team,
>>>>>
>>>>> Please review fix
>>>>> http://cr.openjdk.java.net/~azvegint/jdk/8/8029263/webrev.00/
>>>>> for
>>>>> https://bugs.openjdk.java.net/browse/JDK-8029263
>>>>>
>>>>> This issue can be observed on Solaris 11 (sparcv9 or x86_64).
>>>>>
>>>>> gtk_show_uri() documentation[1] says:
>>>>>> The uri must be of a form understood by GIO (i.e. you need to install
>>>>>> gvfs to get support for uri schemes such as http:// or ftp://, as
>>>>>> only
>>>>>> local files are handled by GIO itself).
>>>>> However it looks like that Solaris 11 supports gvfs for 32-bit
>>>>> applications only by default. gtk_show_uri() returns "Operation not
>>>>> supported" error message  for 64-bit applications
>>>>> for schemes other than file://.
>>>>>
>>>>> Since b110 we don't have 32-bit JDK for Solaris anymore, so in most
>>>>> cases only an OPEN action is available(file://).
>>>>>
>>>>> Currently I am unable to find any robust way to determine do we have
>>>>> full gvfs support to handle URIs like mail:, http:// or not.
>>>>>
>>>>> We can use g_vfs_get_supported_uri_schemes()[2] and assume that we
>>>>> have
>>>>> full gvfs support if http:// scheme is present.
>>>>> But this function depends on a DBUS_SESSION_BUS_ADDRESS environment
>>>>> variable, which can be stripped off in some tests
>>>>> (in this case only file:// scheme will be returned).
>>>>>
>>>>>
>>>>> Old gnome_url_show() will not work  too due to lack of 64-bit
>>>>> libgnomevfs-2.0 library.
>>>>>
>>>>>
>>>>>
>>>>> [1]
>>>>> https://developer.gnome.org/gtk2/stable/gtk2-Filesystem-utilities.html#gtk-show-uri
>>>>>
>>>>>
>>>>> [2]
>>>>> https://developer.gnome.org/gio/stable/GVfs.html#g-vfs-get-supported-uri-schemes
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>>
>>>>> Alexander.
>>>>>
>>>
>