On Mon, 24 Feb 2025 20:10:58 GMT, Alexander Zvegintsev <azveg...@openjdk.org> wrote:
>> Alisen Chung has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 22 commits: >> >> - merge >> - update specifications, replace default values in specifications with >> links to default var >> - update glide in test >> - merge >> - fix test with removed robot.glide using points >> - add code tag to specifications in Robot >> - fix syncdelay in ER >> - removing lesser used overridden methods >> - Merge branch 'master' of https://github.com/openjdk/jdk into 8150564 >> - update specification to public default fields, add waitforidle exceptions >> to specifications >> - ... and 12 more: https://git.openjdk.org/jdk/compare/10791477...30ca6a6b > > src/java.desktop/share/classes/java/awt/Robot.java line 130: > >> 128: >> 129: /** >> 130: * Default 20 milliseconds delay for mouse {@code click} and > > Suggestion: > > * Default delay for mouse {@code click} and > > > There is no need to specify the exact value in the documentation, in case of > something it will be much easier to change it later on. Be aware that values of static fields are automatically documented. eg see (https://docs.oracle.com/en/java/javase/21/docs/api/constant-values.html#java.awt.Font.BOLD) And an app really could use knowing what the default is. And these defaults have been battle-tested in ExtendedRobot > src/java.desktop/share/classes/java/awt/Robot.java line 138: > >> 136: * Default 2 pixel step length for mouse {@code glide}. >> 137: */ >> 138: public static final int DEFAULT_STEP_LENGTH = 2; > > Do we want to make the `DEFAULT_DELAY` and `DEFAULT_STEP_LENGTH` configurable? glide has over-rides step length > src/java.desktop/share/classes/java/awt/Robot.java line 792: > >> 790: * and {@code mouseRelease}. Invokes {@code waitForIdle} with a >> default {@link #DEFAULT_DELAY delay} after >> 791: * {@code mousePress} and {@code mouseRelease} calls. For specifics >> on valid inputs please see >> 792: * {@link java.awt.Robot#mousePress(int)}. > > It's discussable, and I may be wrong, but I'm not a fan of documentation that > is very specific about its implementation. > I prefer the one that was before in the `ExtendedRobot`. > > > Clicks mouse button(s) by calling {@link #mousePress(int)} and {@link > #mouseRelease(int)} methods I understand the concern but in this case it is more or less explanation of what the method does. Perhaps it can be adjusted or made in to an apiNote or whatever is best. > src/java.desktop/share/classes/java/awt/Robot.java line 924: > >> 922: x += tDx; >> 923: y += tDy; >> 924: mouseMove((int)x, (int)y); > > `mouseMove` can throw an `IllegalThreadStateException` under certain > circumstances. yes should be added to all glide API methods ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r2080487052 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r2080487420 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r2080490231 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r2080491661
