8314070: javax.print: Support IPP output-bin attribute extension
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
OutputBinclass are based on Internet Printing Protocol (IPP): “output-bin” attribute extension document. -
CUPSPrinter.getOutputBins(String printer)method uses PPDppdFindOption(..., "OutputBin")function to get supported output bins for the given printer on native level. - The fix propagates the
OutputBinattribute from the printer job attributes toNSPrintInfoprint settings withOutputBinkey 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.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [x] Change requires CSR request JDK-8331601 to be approved
Warning
⚠️ Found leading lowercase letter in issue title for 8314070: javax.print: Support IPP output-bin attribute extension
Issues
- JDK-8314070: javax.print: Support IPP output-bin attribute extension (Enhancement - P4)
- JDK-8331601: javax.print: Support IPP output-bin attribute extension (CSR)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16166/head:pull/16166
$ git checkout pull/16166
Update a local copy of the PR:
$ git checkout pull/16166
$ git pull https://git.openjdk.org/jdk.git pull/16166/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 16166
View PR using the GUI difftool:
$ git pr show -t 16166
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16166.diff
Webrev
:wave: Welcome back alexsch! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@AlexanderScherbatiy The following label will be automatically applied to this pull request:
-
client
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 16: Full - Incremental (1d786c10)
- 15: Full - Incremental (b0ca25c9)
- 14: Full - Incremental (c3d3295e)
- 13: Full - Incremental (c3034f37)
- 12: Full - Incremental (ca48cc85)
- 11: Full - Incremental (ff0db22c)
- 10: Full - Incremental (812ae8b9)
- 09: Full - Incremental (3855d0af)
- 08: Full - Incremental (96e1f532)
- 07: Full - Incremental (23a8149a)
- 06: Full - Incremental (7a0c6579)
- 05: Full (84f09802)
- 04: Full - Incremental (45f953cf)
- 03: Full (25e3ffa1)
- 02: Full - Incremental (7fa504a1)
- 01: Full - Incremental (847059e9)
- 00: Full (e1fa32f8)
Why did you file a new RFE when you know about the existing one ? Close your new one as a dup. and redirect everything to the existing one. Also I don't see any UI work to enhance the dialog so it can be selected. And why restrict it to macOS ? Is it an issue of testing ?
Why did you file a new RFE when you know about the existing one ? Close your new one as a dup. and redirect everything to the existing one. Also I don't see any UI work to enhance the dialog so it can be selected. And why restrict it to macOS ? Is it an issue of testing ?
My idea was to use JDK-8314070 as an umbrella and provide fixes for macOS, Linux, and Windows separately. While the OutputBin public API is not fixed any change in it would require to re-test the fix on all 3 platforms. The fix for the UI dialog requires an additional test with a printer dialog so there would be 2 tests for each platform for re-testing for each essential change in the fix.
Why did you file a new RFE when you know about the existing one ? Close your new one as a dup. and redirect everything to the existing one. Also I don't see any UI work to enhance the dialog so it can be selected. And why restrict it to macOS ? Is it an issue of testing ?
My idea was to use JDK-8314070 as an umbrella and provide fixes for macOS, Linux, and Windows separately. While the OutputBin public API is not fixed any change in it would require to re-test the fix on all 3 platforms. The fix for the UI dialog requires an additional test with a printer dialog so there would be 2 tests for each platform for re-testing for each essential change in the fix.
I'm struggling to follow the logic. This change clearly contains the public API which is the first thing you need. Burying it in the fix for one platform that happens to be the first one you coded isn't right.
If you really want to stagger it then proceed as follows
- Add just the new API class under the existing RFE
- Add this macOS support for it under this ID
- Add the Swing dialog UI support for selecting the bin - easier once you have one implementation
- Add Linux support (CUPS, so should be similar to macOS)
- Add Windows support, but SFAIK GDI has no support for this, so likely impossible to do.
@AlexanderScherbatiy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
This is some investigation which I did for output bin attribute extension support on Linux.
PostScript Language Reference manual third edition defines OutputType page device parameter with type string which represents special attributes of the output destination.
The example of OutputType usage I have found in CUPS PPD Extensions.
OutputType specifies the output type name, for example:
<</OutputType (Photo)>>setpagedevice
This is my implementation of adding OutputType parameter in PSPrinterJob https://github.com/AlexanderScherbatiy/jdk/commit/92ce5b9e23e0338c0d1b49a653dbc67e0512bdc4 added to a separate branch
protected void startDoc() throws PrinterException {
...
String outputBin = getOutputBinValue(outputBinAttr);
if (outputBin != null) {
mPSStream.print(" /OutputType (" + outputBin + ") ");
}
I tested it with Kyocera ECOSYS M8130cidn printer using Ubuntu 20.04.1 LTS.
The ppdFindOption(ppd, "OutputBin") function from CUPSfuncs returns two output bins for the Kyocera ECOSYS M8130cidn:
text: Top, choice: Top
text: Stacker 1, choice: Stacker1
The result was that a page is printed to the same output tray (Top) no matter Top or Stacker1 output bin is used for OutputType post script parameter.
First, I checked that printing a pdf file from the Unix PDF Viewer app using the system print dialog and choosing different output trays really prints pages to different output trays.
Second, I copied post script files sent by java program for printing from /var/spool/cups directory.
The post script file for Top output bin contains:
%!PS-Adobe-3.0
...
%%BeginSetup
<< /PageSize [595.2755737304688 841.8897705078125] /DeferredMediaSelection true /ImagingBBox null /ManualFeed false /NumCopies 1 /OutputType (Top) >> setpagedevice
%%EndSetup
which /OutputType (Top) setting looks correct.
And for the Stacker1 output bin:
%%BeginSetup
<< /PageSize [595.2755737304688 841.8897705078125] /DeferredMediaSelection true /ImagingBBox null /ManualFeed false /NumCopies 1 /OutputType (Stacker1) >> setpagedevice
%%EndSetup
the setting for output bin is /OutputType (Stacker1).
The file which sends the PDF Viewer for printing includes %PDF-1.5 version and I was not able to find the parameter corresponding to the selected output bin even if I convert the file from pdf to ps format using pdf2ps tool which generates file in %!PS-Adobe-3.0 fromat.
It is not clear for me why /OutputType (Stacker1) parameter does not work and is there a way to take the output bin parameter which Linux native printer dialog use to print a document to the Stacker 1 tray for the Kyocera ECOSYS M8130cidn printer.
One more question which I have relates to the java common print dialog. It needs to contain list of available output bins. Is it safe to assume that the first output bin retrieved by ppdFindOption(ppd, "OutputBin") is the default one? Or should there be added one more Default option for the default output bin selection?
@AlexanderScherbatiy this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout JDK-8318023
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
/issue add 8314070
@AlexanderScherbatiy
Adding additional issue to issue list: 8314070: javax.print: Support IPP output-bin attribute extension.
The common print dialog is updated to show supported output bins.
-
OutputPanelis added to theServiceDialogto show list of supported output bins in the ComboBox. -
order.outputandlabel.outputbinsproperties are added toserviceui.properties. -
CPrinterJob.nsPrintInfoToJavaPrinterJob()method is updated to add the selected output bin from the native print dialog to the printer job attributes. -
OutputBinAttributePrintDialogTestmanual test that checks only printing to the first and the last supported output bins with common and native print dialog is added.
Common print dialog with the fix which shows list of supported output bins on macOS:
The same dialog with disabled output trays when the list of output bins is empty:
The same dialog without the fix:
There is the comment on social.msdn.microsoft.com forum that wingdi does not support setting output bin attribute programmatically:
"'Output tray" is a printer-specific property (note that you will only see such an option
in the printing interface for certain printers). There is no way to programmatically set
an output tray, since there is no generic way of doing it via .NET (or even Win32
via the DEVMODE structure). The only way to set it is to allow the user
to choose it via the provided printer dialog for the few printers that provide this capability
(just as you must do via IE or Word).
It is sometimes possible to do this programmatically (without the dialog) for a specific
printer if you know exactly how that printer driver implements the change...but that
is unsupported except possibly from the printer manufacturer.
https://social.msdn.microsoft.com/Forums/en-US/b15eda2f-5353-41fc-b935-078ed0960aee/printing-into-a-specific-output-bintray-net
That is why OutputPanel is excluded from the ServiceDialog on Windows.
@AlexanderScherbatiy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@AlexanderScherbatiy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@AlexanderScherbatiy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@AlexanderScherbatiy This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8314070: javax.print: Support IPP output-bin attribute extension
Reviewed-by: psadhukhan, prr
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 439 new commits pushed to the master branch:
- 6dac8d64527b4e9ade783b99f82fbecd81c426a6: 8332424: Update IANA Language Subtag Registry to Version 2024-05-16
- 9686e804a2b058955ff88149c54a0a7896c0a2eb: 8333103: Re-examine the console provider loading
- 4de620732f03c71fec3e1c233947742d334c88ad: 8333229: Parallel: Rename ParMarkBitMap::_region_start to _heap_start
- 1f9e62904c624b12bd344d2ef3021eb5d3377197: 8333434: IGV: Print loop node for PHASE_BEFORE/AFTER_CLOOPS
- 27af19d921a5cf15f5146471b58961815690b4f2: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}
- 1c514b34c0260823e70f209996ac933a76ac34c2: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized
- d07e530d33360dae687552a6dfbe26408f3fb58e: 8333128: Linux x86_32 configure fail with --with-hsdis=binutils --with-binutils-src
- f0bffbce35bb06e724857e8651dd429c4f9df284: 8333301: Remove static builds using --enable-static-build
- b10158624bd0cfe009f0fe7f2a06ee08e654766b: 8332514: Allow class space size to be larger than 3GB
- 5ed0d52c8424dd2e7f1ac2404e9fabb40c8402b8: 8332936: Test vmTestbase/metaspace/gc/watermark_70_80/TestDescription.java fails with no GC's recorded
- ... and 429 more: https://git.openjdk.org/jdk/compare/b96b38c2c9a310d5fe49b2eda3e113a71223c7d1...master
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
One more question which I have relates to the java common print dialog. It needs to contain list of available output bins. Is it safe to assume that the first output bin retrieved by
ppdFindOption(ppd, "OutputBin")is the default one? Or should there be added one moreDefaultoption for the default output bin selection?
You should definitely query for the default. But then how to use it ..
The way the dialog works is that there's an initial set of attributes (needed if the user cancels) and the "asCurrent" which starts out as a copy of that inital set. Anything not in the initial set, or if the default value is not supported, we look for the default value - assuming that the category is supported at all - and that's what is the selected item in the dialog UI but that does NOT cause the default to be copied to asCurrent. So if the user selects a different printer, the dialog gets the new printer's default - if any is supported. The general idea being anything they didn't touch should mean "use the default".
But if the user explicitly selects a value it is added to asCurrent and carried over to the next printer, so long as it is a supported value there, otherwise its discarded (at least that's the intended idea). So if you don't know the default, it isn't very easy to do the right thing.
The updated fix adds the default output bin value on the print dialog as the latest empty line which is selected if the output bin has not been provided as the asCurrent.
It is not clear for me is it ok to leave this value as empty or it should have name like Default. In later case it needs to translate the name to other languages as well.
This is getting close enough that it is time to start on the CSR, since I think the API part is settled now.
/csr
/Reviewers 2
@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@AlexanderScherbatiy please create a CSR request for issue JDK-8314070 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.
I have created the CSR https://bugs.openjdk.org/browse/JDK-8331601 and assigned it to you to fill in. Once you've done that I can review it.
Note that RDP1 for JDK 23 is 6/6/24 - https://openjdk.org/projects/jdk/23/ so there's still time to get it into JDK 23, so you long as you get the CSR submitted soon.
I added the OutputBin class and package-info.java changes to the CSR javax.print: Support IPP output-bin attribute extension description: https://bugs.openjdk.org/browse/JDK-8331601
The OutputBin class has two myStringTable and myEnumValueTable tables. They are private but their values are accessed through the public API. Should myStringTable and myEnumValueTable tables be mentioned in the CSR?
Should the changes in the common print dialog (the output bin label and the output bin combo-box) and new properties from the serviceui.properties be described in the CSR?
Mailing list message from Philip Race on client-libs-dev:
On 5/6/24 11:08 AM, Alexander Scherbatiy wrote:
On Thu, 2 May 2024 18:30:23 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:
Change 'Page Setup' to 'Appearance' tab in the OutputBinAttributePrintDialogTest I added the `OutputBin` class and package-info.java changes to the CSR `javax.print: Support IPP output-bin attribute extension` description: https://bugs.openjdk.org/browse/JDK-8331601
The package-info text isn't spec. relevant, so I removed it from the CSR.
The `OutputBin` class has two `myStringTable` and `myEnumValueTable` tables. They are private but their values are accessed through the public API. Should `myStringTable` and `myEnumValueTable` tables be mentioned in the CSR?
I didn't think it was necessary, so I edited this out. In general I cut out anything that wasn't spec and wasn't too hard to edit away.
Should the changes in the common print dialog (the output bin label and the output bin combo-box) and new properties from the `serviceui.properties` be described in the CSR?
No need that I can see.
Good that you got it in and finalized this week.
-phil
@AlexanderScherbatiy did you see the comment in the CSR about sealed classes ? I had originally suggested in this PR (search for a comment from a while ago) and I don't know if you saw it there either.
@AlexanderScherbatiy did you see the comment in the CSR about sealed classes ? I had originally suggested in this PR (search for a comment from a while ago) and I don't know if you saw it there either.
@prrace There are two things which are not clear for me.
The first one is that a user can extend OutputBin class and provide its own implementation.
Should it be possible to the user provide output bins other than supported ones?
The another question relates to CustomOutputBin class which is used to provide a list of supported output bins and which is not public.
To make OutputBin class sealed it needs to add CustomOutputBin class to OutputBin class permits list:
public sealed class OutputBin extends EnumSyntax implements PrintRequestAttribute, PrintJobAttribute
permits sun.print.CustomOutputBin {
}
public final class CustomOutputBin extends OutputBin {
}
Is it good to use a non public class in the public class permits list?
I made docs for the jdk with sealed OutputBin class.
It has the sealed keyword in the javadoc but mentions nothing about internal CustomOutputBin class.
images/docs/api/java.desktop/javax/print/attribute/standard/OutputBin.html:
public sealed class OutputBin
extends EnumSyntax
implements PrintRequestAttribute, PrintJobAttribute
@prrace
I have updated the fix to make the OutputBin class sealed and CustomOutputBin class final.
The OutputBin class permits internal sun.print.CustomOutputBin class.
The OutputBin javadoc that Implementation- or site-defined names for an output bin kind attribute may also be created by defining a subclass of class {@code OutputBin}. is removed.
If it is an applicable change I will update the CSR as well.