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

[Phil Race]

On Wed, 13 Dec 2023 15:58:03 GMT, Alexander Scherbatiy <alexsch at openjdk.org> wrote:

>> The fix adds new public `OutputBin` print attribute class which allow to set a printer output bin in a `PrinterJob` class. The corresponding internal `CustomOutputBin` class is added as well.
>> 
>> - Constants used in `OutputBin` class are based on [Internet Printing Protocol (IPP): “output-bin” attribute extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf) document.
>> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD  `ppdFindOption(..., "OutputBin")` function to get supported output bins for the given printer on native level.
>> - The fix propagates the `OutputBin` attribute from the printer job attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS.
>> 
>> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice): 
>> - Printer settings, None
>> - Inner tray, INNERTRAY
>> - Separator tray, SEPARATORTRAY
>> - Finisher (face-down), Main
>> 
>> if `Printer settings`, `Inner tray`, or `Finisher (face-down)` CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is printed to the Main tray of the  `Kyocera ECOSYS M8130cidn` printer. If `Separator tray` is used a page is printed to the Separator tray. This is consistent with the printer behavior when a native print dialog is used from a native Preview app to print a document on macOS.
>
> Alexander Scherbatiy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add output bins support to the common print dialog

src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java line 53:

> 51:  * attribute value.
> 52:  */
> 53: public class OutputBin extends EnumSyntax

Do we see a reason for applications to subclass OutputBin ?
If not perhaps it could be a sealed class.
Of course given that this isn't the case for all the other attributes it probably doesn't add much.

src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java line 55:

> 53: public class OutputBin extends EnumSyntax
> 54:         implements DocAttribute, PrintRequestAttribute, PrintJobAttribute {
> 55: 

Not sure I see this as a DocAttribute. We have a couple of dubious DocAttributes, but I don't see the bin the output is sent to as a property of the document.

Also this class needs to be added to the table here :
https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/javax/print/attribute/standard/package-summary.html

src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java line 57:

> 55: 
> 56:     /**
> 57:      * Use serialVersionUID from JDK 1.4 for interoperability.

Obviously not the case here. I think you should just delete this comment.

src/java.desktop/share/classes/sun/print/CustomOutputBin.java line 60:

> 58:      */
> 59:     public static CustomOutputBin createOutputBin(String name, String choice) {
> 60:         for(CustomOutputBin bin: customEnumTable) {

for( -> for (

and perhaps this method should be synchronized.

BTW I assume this loop is meant to be equivalent to what you did here https://github.com/openjdk/jdk/pull/16167/files ?


Not sure where else to put this question but in the PR description you wrote
The fix was tested on Kyocera ECOSYS M8130cidn printer where ppdFindOption(..., "OutputBin") call returns 4 output bins (text, choice):

Printer settings, None
Inner tray, INNERTRAY
Separator tray, SEPARATORTRAY
Finisher (face-down), Main
And (eg)
If Separator tray is used a page is printed to the Separator tray.

I would have assumed "SEPARATORTRAY" was the keyword name you would specify to the job and "Separator Tray" is just a description - which could be used in a UI menu.
Am I missing something ?

src/java.desktop/share/classes/sun/print/CustomOutputBin.java line 69:

> 67: 
> 68:     /**
> 69:      * Use serialVersionUID from JDK 1.4 for interoperability.

delete comment

src/java.desktop/share/classes/sun/print/ServiceDialog.java line 1333:

> 1331: 
> 1332:         private MediaPanel pnlMedia;
> 1333:         private OutputPanel pnlOutput;

I've mulled this over and I'm not sure that Page Setup is where this should appear. I suspect you considered it analogous to  Source but IPP considers input tray to be a Media.

I don't think everything is logical about the rest of how things are divided up/named either but I just don't see it as a Page Setup option.

I think it might be better placed under Sides + Job Attributes on the so-called "Appearance" tab.

src/java.desktop/share/classes/sun/print/resources/serviceui.properties line 32:

> 30: border.jobattributes=Job Attributes
> 31: border.media=Media
> 32: border.output=Output

You only updated the default file.
You MUST update all of them, even if you just use English as above. If you don't do this and you are in (say) Germany with language set to German then the dialog will fail like this

Exception in thread "AWT-EventQueue-0" java.lang.Error: Fatal: Resource for ServiceUI is broken; there is no border.output key in resource

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564267144
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564264748
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564262491
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564267453
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564269482
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564302988
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1564296838