<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)

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Fri Dec 13 05:30:29 PST 2013 

Hi, Alexander.
The fix looks good to me too.

On 13.12.2013 16:49, Anthony Petrov wrote:
> Thanks. The fix looks good to me now.
>
> -- 
> best regards,
> Anthony
>
> On 12/13/2013 04:44 PM, Alexander Zvegintsev wrote:
>> Sure,  webrev updated in place.
>> I also updated the RFE description.
>>
>> Thanks,
>>
>> Alexander.
>>
>> On 12/13/2013 04:33 PM, Anthony Petrov wrote:
>>> 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.
>>>>>>>>
>>>>>>
>>>>
>>


-- 
Best regards, Sergey.

More information about the awt-dev mailing list