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.
