On Wed, 26 Jun 2024 23:42:16 GMT, Alisen Chung <ach...@openjdk.org> wrote:
>> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 640:
>>
>>> 638: HGLOBAL oldG = AwtPrintControl::getPrintHDMode(env, self);
>>> 639: if (setup.hDevMode != oldG) {
>>> 640: AwtPrintControl::setPrintHDMode(env, self, setup.hDevMode);
>>
>> what does setPrintHDMode and setPrintHDName do? do they need to be called as
>> part of cleanup even if there is an exception and we return JNI_FALSE?
>
> i think this code preserves the printer settings after the dialog closes (and
> only on success before this fix), so is it possible in the case the printer
> is removed and causes an exception, we shouldn't preserve these settings?
It's a good question…
Yes, the code saves (or updates) the printer name and mode.
These lines of code were called in cases where OK or Cancel is clicked in the
print dialog. However, it was skipped in case of an error, just like
`CheckUninstallModalHook`.
I also feel that these lines belong in the successful case, right before
`return JNI_TRUE`, but I could quickly find any confirmation whether
`setup.hDevMode` or `setup.hDevNames` can be modified. For this reason, I
preserved the previous code flow.
I presume that neither `setup.hDevMode` or `setup.hDevNames` changes if the
user clicks Cancel in `::PageSetupDlg`. It still leaves the question open what
happens when the user clicks OK but we catch an error inside `if (ret)`. The
old handles stored inside `self` could have become invalid, so it's safer to
update the handles in `self` even if an error occurs.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19867#discussion_r1656892141
