On Thu, 4 May 2023 16:26:47 GMT, Alexander Zvegintsev <azveg...@openjdk.org> wrote:
> We need to relax the java.awt.Robot specification according to the latest > operating system trends. > This should at least cover the case of Wayland, which has changed many > familiar concepts in Linux. > > https://bugs.openjdk.org/browse/JDK-8280982 [Wayland] [XWayland] > java.awt.Robot taking screenshots > https://bugs.openjdk.org/browse/JDK-8280995 [XWayland] Robot.mouseMove does > not visually move mouse cursor > https://bugs.openjdk.org/browse/JDK-8280990 [XWayland] XTest emulated mouse > click does not bring window to front. > https://bugs.openjdk.org/browse/JDK-8280988 [XWayland] Click on title to > request focus test failures Changes requested by prr (Reviewer). src/java.desktop/share/classes/java/awt/Robot.java line 79: > 77: * (e.g. interacting with window decorations, trying to change the > 78: * window stacking order with the mouse or keyboard shortcuts) > 79: * We won't be running in compatibility mode when we have a pure wayland port .. I think it would be nice if we could get this wording done in such a way that it applies to both .. and even covers the situation on macOS too. Also this sort of overlooks that we don't support wayland today and won't be able to do screen capture in "default" mode .. so its really tricky to know whether to introduce wording like this. Here's my first stab. This is pure SPEC, not "notes" or "impl" <pre> * Platforms and desktop environments may impose restrictions or limitations * on the access required to implement all functionality in the Robot class. * For example * + preventing access to the contents of any part of a desktop or Window on the desktop that is not owned by the running application * + treating window decorations as non-owned content. * + ignoring or limiting specific requests to manipulate windows. * + ignoring or limiting specific requests for Robot generated (synthesized) events related to keyboard and mouse etc. * + requiring specific or global permissions to any access to window contents, even application owned content, * or to perform even limited synthesizing of events. * * The Robot API specification requires that approvals for these be granted for full operation. * If they are not granted, the API will be degraded as discussed here. * Relevant specific API methods may document more specific limitations and requirements. * Depending on the policies of the desktop environment, the approvals mentioned above may + be required every time + or persistent for the lifetime of an application, + or persistent across multiple user desktop sessions + be fine-grained permissions + be associated with a specific binary application, or a class of binary applications. When such approvals need to given interactively, it may impede the normal operation of the application until approved, and if approval is denied or not possible, or cannot be made persistent then it will degrade the functionality of this class and in turn any part of the operation of the application which is dependent on it. </pre> src/java.desktop/share/classes/java/awt/Robot.java line 399: > 397: * Returns the color of a pixel at the given screen coordinates. > 398: * > 399: * @implNote On Linux systems using Wayland, permission may be > requested We need more than an implNote here. It needs to be spec. Also the words about system dialog don't belong. There may be other ways. And "may be black" ? That's not very useful. Either guarantee its black or say "unspecified". Is black guaranteed today if we fail to grab it ? But also it seems to contradict the SecurityException docs .. how do you get black if an exception is thrown ? src/java.desktop/share/classes/java/awt/Robot.java line 416: > 414: * @throws SecurityException if {@code readDisplayPixels} permission > 415: * is not granted, or user has denied screen capture for all > 416: * screens * SecurityException if (1) {@code readDisplayPixels} permission is not granted, or (2) user has denied screen capture for all screens (1) is the java SecurityManager scenario. Since that's not yet removed, we do need those words, so OK (2) Is there some subtlety about "ALL" screens ? Do you really mean all screens which are in the area being captured ? Does that also imply you can get a partial capture if it spans 2 screens and you only have permission for one of them ? I'm not sure how useful that is in the "screenCapture" case and its really odd for the single pixel capture .. which surely is only on one screen ? We do also need to think forward to a pure wayland scenario - any differences there. ? The word "Wayland" shouldn't appear in the spec My proposal - <pre> If the desktop environment requires that permissions be granted to capture screen content, and the required permissions are not granted, then a {@code SecurityException} may be thrown, or the contents of the returned {@code BufferedImage} are undefined. @apiNote: It is recommended to avoid calling this method on the AWT Event Dispatch Thread since screen capture may be a lengthy operation, particularly if acquiring permissions is needed and involves user interaction. @throws SecurityException if {@code readDisplayPixels} permission * is not granted, or user has denied screen capture for all * required screens </pre> ==== I added the word REQUIRED here, since if we only need screen 1 contents, surely it doesn't matter if we don't have screen 2 permission ? src/java.desktop/share/classes/java/awt/Robot.java line 640: > 638: * a new permission from the user on applicable platforms. > 639: */ > 640: public void revokeScreenCapturePermission() { I do not understand why we need this method. Also we are separating spec. relaxation into this bug / PR which BTW means it needs a new bug ID. But this would be a new API, so it would need to go along with the code that implements it. Perhaps that's why you added the default method below .. but it isn't really the same thing. ------------- PR Review: https://git.openjdk.org/jdk/pull/13809#pullrequestreview-1417479332 PR Review Comment: https://git.openjdk.org/jdk/pull/13809#discussion_r1187970612 PR Review Comment: https://git.openjdk.org/jdk/pull/13809#discussion_r1187882524 PR Review Comment: https://git.openjdk.org/jdk/pull/13809#discussion_r1187889861 PR Review Comment: https://git.openjdk.org/jdk/pull/13809#discussion_r1187875121
