RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v14]

Alexander Scherbatiy alexsch at openjdk.org
Tue May 28 16:56:16 UTC 2024


On Tue, 28 May 2024 07:54:40 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Alexander Scherbatiy has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - Fix types in tests
>>  - Fix typos in tests
>>  - Check if OutputBin category supported in tests
>>  - Add automated CheckSupportedOutputBinsTest test
>
> src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java line 49:
> 
>> 47: import javax.print.attribute.standard.PageRanges;
>> 48: import javax.print.attribute.standard.Sides;
>> 49: import javax.print.attribute.standard.OutputBin;
> 
> Please sort the import...Guess OutputBin should come before PageRanges

Updated.

> src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 93:
> 
>> 91: import javax.print.attribute.standard.SheetCollate;
>> 92: import javax.print.attribute.standard.Sides;
>> 93: import javax.print.attribute.standard.OutputBin;
> 
> Sort the import..OutputBin should be placed before PageRanges

Updated.

> src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 1283:
> 
>> 1281:         if (!isSupportedValue(outputBinAttr,  attributes)) {
>> 1282:             outputBinAttr = null;
>> 1283:         }
> 
> Should we set it to null if user-set output-bin attribute is not among supported values
> or
> should we set it to default
> as is done for printer-resolution and others

It seems that setting the outputBinAttr to null should be fine as it means that no OutputBin value is provided by jdk and the printer will use the default one.

> src/java.desktop/share/classes/sun/print/ServiceDialog.java line 2927:
> 
>> 2925:             lblOutput.setEnabled(outputEnabled);
>> 2926: 
>> 2927:             cbOutput.addItemListener(this);
> 
> If Attribute category is unsupported, the panel will be disabled, in that case, is there any need to add itemlistener? I guess we can make it conditional to category being supported

The fix is updated to add the itemListener only when the cbOutput JComboBox is enabled.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1617619945
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1617620138
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1617622196
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1617624070


More information about the client-libs-dev mailing list