On Wed, 19 Jun 2024 08:30:42 GMT, Prasanta Sadhukhan <[email protected]> wrote:
>> On cancelling PageDialog, same PageFormat object should be returned which >> stopped working after >> [JDK-8307160](https://bugs.openjdk.org/browse/JDK-8307160). >> Fix is made to reinstate "doIt" flag removed in JDK-8307160 so that correct >> value is returned from PageDialog.show action.. >> An automated printing testcase is created since the issue was caught by >> manual test and so having another manual test run the risk of not being >> executed during CI testing.. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Test fix Changes requested by aivanov (Reviewer). src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 694: > 692: ::GlobalUnlock(setup.hDevMode); > 693: } > 694: doIt = JNI_TRUE; Another option would be to return `JNI_TRUE` here and to return `JNI_FALSE` in the end of the function. src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 697: > 695: } > 696: > 697: AwtDialog::CheckUninstallModalHook(); I wonder why the block of code at lines 697–709 is not needed if `JNI_FALSE` is returned as a result of an error condition. No, it is not directly connected to the bug you're fixing, yet it doesn't look right to me. test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 57: > 55: robot.waitForIdle(); > 56: robot.delay(500); > 57: }); These key presses are sent to whatever window has focus… before the Page Dialog is shown and after it's hidden. Is pressing `VK_ESCAPE` not enough? Pressing Esc is the same as clicking Cancel button. test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 60: > 58: t1.start(); > 59: PageFormat newFormat = pj.pageDialog(oldFormat); > 60: if (!newFormat.equals(oldFormat)) { You should interrupt the `t1` thread after `pj.pageDialog` returns to stop robot from sending `keyPress` and `keyRelease` after the dialog is hidden. You can even use `Thread.sleep` instead of `Robot.delay` so that interrupting the thread throws `InterruptedException` and its handler just exits. (`Robot.delay` catches `InterruptedException` and then restores the interrupted flag.) I wonder if any AWT event is sent when the Page Dialog is shown on the screen, an event is more reliable and key presses won't be sent to an arbitrary window in the system. ------------- PR Review: https://git.openjdk.org/jdk/pull/19786#pullrequestreview-2127813477 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645895912 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645900131 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645886899 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645894197
