RFR: 8280994: [XWayland] Drag and Drop does not work in java -> wayland app direction [v2]

Prasanta Sadhukhan psadhukhan at openjdk.org
Fri Jun 2 12:10:14 UTC 2023


On Thu, 1 Jun 2023 18:38:29 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

>> This change resolves the issue that drag and drop from a Java application to a native Wayland application (such as gedit) is not possible on Linux.
>> 
>> Our code it quite picky for the drop target, and requires it to be a top level window with the WM_STATE property set, whereas the XWayland server provides dummy windows without this property.
>> So now we make an exclusion for this case.
>> 
>> After the fix it successfully passes previously failed related manual jtreg/conformance tests on Wayland.
>> Other tests are also passed.
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix toolkit if condition

src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java line 410:

> 408:         Toolkit toolkit = Toolkit.getDefaultToolkit();
> 409:         if (!(toolkit instanceof SunToolkit)
> 410:                 || !((SunToolkit) toolkit).isRunningOnWayland()) {

Although it's not related to this fix, but `SunToolkit.isRunningOnWayland` does not have javadoc/comment, maybe we can think of adding one there since we added a javadoc-style comment for this private method in non-spec class

src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java line 418:

> 416:                         XDnDConstants.XA_XdndAware,
> 417:                         0, 1, false,
> 418:                         XConstants.AnyPropertyType);

I guess formatting can follow this style as the newlines are starting abruptly it seems
https://github.com/openjdk/jdk/blob/73e7af9e28805dda25f91fd509e3860d8586ad9f/src/java.desktop/unix/classes/sun/awt/X11/XDnDDragSourceProtocol.java#L196-L197

src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java line 422:

> 420:         try {
> 421:             int status = wpg.execute(XErrorHandler
> 422:                     .IgnoreBadWindowHandler.getInstance());

I guess better will be to format so that `IgnoreBadWindowHandler` is placed below `XErrorHandler` as it belongs to that class

src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java line 426:

> 424:             return status == XConstants.Success
> 425:                     && wpg.getData() != 0
> 426:                     && wpg.getActualType() == XAtom.XA_ATOM;

Better to align just below s of `status`

src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java line 448:

> 446:         }
> 447: 
> 448:         if (isXWaylandDndAwareWindow(window)) {

https://github.com/openjdk/jdk/blob/ec4493f5273746fdbc2a9c9451c15050d04730d2/src/java.desktop/unix/classes/sun/awt/X11/XDragSourceContextPeer.java#L441-L444

Is it not required to do `win != 0 && isXWaylandDndAwareWindow()`  since we are returning value from here too?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14266#discussion_r1214096897
PR Review Comment: https://git.openjdk.org/jdk/pull/14266#discussion_r1214105030
PR Review Comment: https://git.openjdk.org/jdk/pull/14266#discussion_r1214124414
PR Review Comment: https://git.openjdk.org/jdk/pull/14266#discussion_r1214126806
PR Review Comment: https://git.openjdk.org/jdk/pull/14266#discussion_r1214122682



More information about the client-libs-dev mailing list