On Wed, 13 Dec 2023 15:58:03 GMT, Alexander Scherbatiy <alex...@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
