On Tue, 25 Feb 2025 14:55:41 GMT, GennadiyKrivoshein <d...@openjdk.org> wrote:
>> Fix for https://bugs.openjdk.org/browse/JDK-8349350. It's impossible to use >> more that one print option. >> >> **Reason of the bug**: >> execCmd array uses one index per print flag, but 'OPTIONS' flag can use two >> indexes for the options. >> >> **Fix description**: >> make the size of the execCmd array dependent on the number of options. >> >> **Test**: >> new test PrintExecCmdOptionTest.java created to check execution with >> multiple options. (run on MacOS, Windows and linux x86_64) > > GennadiyKrivoshein has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Merge branch 'openjdk:master' into print_options_idx_out_of_rng > - remove code duplication > - replace regexp s+ with space > - use array for option args > - Fix ArrayIndexOutOfBoundsException at PSPrinterJob printExecCmd I notice that UnixPrintJob is even worse .. it would have the same problem except it doesn't even try to iterate over options, even though they can be specified ! I think it makes sense to fix that too. src/java.desktop/share/classes/sun/print/PSPrinterJob.java line 1579: > 1577: if (options != null && !options.isEmpty()) { > 1578: optionArgs = options.trim().split(" "); > 1579: ncomps+=optionArgs.length; I would have gone for the simpler one line fix of ncomps+=options.trim().split(" ").length test/jdk/javax/print/PrintExecCmdOptionTest.java line 59: > 57: MediaTray mediaTray = null; > 58: for (PrintService ps : printServices) { > 59: Media[] medias = (Media[]) ps. It looks to me as if this loop needs to reset mediaTray = null. Otherwise if printer 'N' has a mediaTray, printer 'N+1' will inherit it, even if it does not have one. ------------- PR Review: https://git.openjdk.org/jdk/pull/23457#pullrequestreview-2662591372 PR Review Comment: https://git.openjdk.org/jdk/pull/23457#discussion_r1982255789 PR Review Comment: https://git.openjdk.org/jdk/pull/23457#discussion_r1982242904