On Thu, 6 Feb 2025 22:03:54 GMT, Alisen Chung <ach...@openjdk.org> wrote:
>> Some useful methods in ExtendedRobot should be migrated into Robot itself so >> that ExtendedRobot can be removed in the future. The tests using >> ExtendedRobot for these migrated methods are changed to use only Robot >> (removing unnecessary building of ExtendedRobot). > > 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. src/java.desktop/share/classes/java/awt/Robot.java line 136: > 134: > 135: /** > 136: * Default 2 pixel step length for mouse {@code glide}. Suggestion: * Default pixel step length for mouse {@code glide}. Same here 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? 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 src/java.desktop/share/classes/java/awt/Robot.java line 858: > 856: * @since 25 > 857: */ > 858: public void glide(int x, int y) { I don't see the `public void glide(Point dest)` and `public void glide(Point src, Point dest)` added, it may be convenient in some cases. 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. test/jdk/java/awt/Component/PaintAll/PaintAll.java line 51: > 49: @summary Test Component.paintAll() method > 50: @author sergey.bylok...@oracle.com: area=awt.component > 51: @library /lib/client/ It's not needed anymore, is it? Suggestion: test/jdk/lib/client/ExtendedRobot.java line 182: > 180: @Override > 181: public synchronized void waitForIdle() { > 182: waitForIdle(syncDelay); This `waitForIdle(500)` is no longer called by tests(as it uses regular `java.awt.Robot#waitForIdle()`, so I assume you have verified that automated testing looks good. (I haven't gone through all the tests yet) test/jdk/lib/client/ExtendedRobot.java line 368: > 366: * @see java.awt.event.KeyEvent > 367: */ > 368: public void type(char c) { Those `type(char...` are not migrated also. At least one test uses it: https://github.com/alisenchung/jdk/blob/8150564/test/jdk/java/awt/Window/ShapedAndTranslucentWindows/ShapedTranslucentWindowClick.java#L178 With your change, it now calls `type(int)` directly, which has a different implementation. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968352863 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968353101 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968417841 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968386427 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968425006 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968422144 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968431117 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968414517 PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968435956
