[OpenJDK 2D-Dev] [9] RFR JDK-6357887: selected printertray is ignored under linux

[Vadim Pakhnushev]

In the PSPrinterJob.java customTray.getChoiceName() is not checked for 
null but in the UnixPrintJob it is checked.
Could this be a problem?

The getMediaTray method in the test could be simplified to
for (Media m : media) {
     if (m instanceof MediaTray) {
         if (m.toString().trim()...) {
           return (MediaTray)m;
         }
     }
}
return null;

Vadim

On 25.08.2016 16:25, Philip Race wrote:
> +1
>
> -phil
>
> On 8/24/16, 11:27 PM, Prasanta Sadhukhan wrote:
>> Thanks Phil for the comments.
>> Modified webrev:
>> http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.02/
>>
>> Regards
>> Prasanta
>> On 8/25/2016 12:05 AM, Phil Race wrote:
>>> In fact what you should be doing here is
>>> Attribute attr = attrs.get(Media.class);
>>> if (attr instanceof CustomMediaTray) ....
>>>
>>> The program below should show that the lookup works so long as the key
>>> is the class understood by the API - not a sub-type.
>>>
>>> -phil.
>>> import javax.print.*;
>>> import javax.print.attribute.*;
>>> import javax.print.attribute.standard.*;
>>>
>>> public class CMT {
>>>
>>>    public static void main(String args[]) throws Exception {
>>>
>>>       PrintService svc = 
>>> PrintServiceLookup.lookupPrintServices(null,null)[0];
>>>       Media[] allMedia =
>>>          (Media[])svc.getSupportedAttributeValues(Media.class, null, 
>>> null);
>>>       PrintRequestAttributeSet aset = new 
>>> HashPrintRequestAttributeSet();
>>>       for (int m=0;m<allMedia.length;m++) {
>>>             Media in = allMedia[m];
>>>             aset.add(in);
>>>             Media out1 = (Media)aset.get(Media.class);
>>>             System.out.println("Class="+in.getClass()+" in="+in+ " 
>>> out="+out1);
>>>             Media out2 = (Media)aset.get(in.getClass());
>>>             System.out.println("Class="+in.getClass()+" in="+in+ " 
>>> out="+out2);
>>>       }
>>>    }
>>>
>>>
>>>
>>> On 8/18/2016 11:20 PM, Prasanta Sadhukhan wrote:
>>>>
>>>>
>>>> On 8/19/2016 3:36 AM, Philip Race wrote:
>>>>> Oh .. right under the covers the map is just a HashMap
>>>>> and the key to the map is the class so just decides if these are
>>>>> equal class objects. Hmm .. I don't know anymore if that was the 
>>>>> original intent
>>>>> but we probably should not mess with it right now just for the 
>>>>> optimisation.
>>>>> But now I am wondering why it has to be CustomMediaTray and not just
>>>>> MediaTray ... although I suspect that in the case of CUPS we are not
>>>>> ever mapping the media to standard ones, so they are always custom.
>>>> Yes, in CUPS 
>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/9f38d4f86e3d/src/java.desktop/unix/classes/sun/print/CUPSPrinter.java#l254
>>>> we instantiate CustomMediaTray (and do not map to standard) so I 
>>>> need to check for CustomMediatray to get the tray instance.
>>>>
>>>> Regards
>>>> Prasanta
>>>>> That might be arise as a problem in the case of your other open 
>>>>> review regarding
>>>>> validating the margins. I'll comment on that in that review but 
>>>>> still surely
>>>>> any MediaTray is what you would want here but it is probably moot if
>>>>> you don't ever get a standard one. Please check into this and confirm
>>>>> what I suspect .. or not ...
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 8/17/16, 11:41 PM, Prasanta Sadhukhan wrote:
>>>>>> MediaTray values are bundled with "Media" attribute so calling 
>>>>>> attributes.get(CustomMediatray.class) returns null.
>>>>>> Modified webrev to get Media attribute and then see if the 
>>>>>> attribute is an instance of CustomMediatray.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.01/
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 8/17/2016 9:09 PM, Philip Race wrote:
>>>>>>> This all looks fine although this can be a simple call to
>>>>>>> attributes.get(CustomMediaTray.class) can't it ?
>>>>>>>
>>>>>>> 502         for (int i=0; i< attrs.length; i++) {
>>>>>>>  503             Attribute attr = attrs[i];
>>>>>>>  504             try {
>>>>>>>  505                 if (attr instanceof CustomMediaTray) {
>>>>>>>  506                     CustomMediaTray customTray = 
>>>>>>> (CustomMediaTray)attr;
>>>>>>>  507                     mOptions = " InputSlot="+ 
>>>>>>> customTray.getChoiceName();
>>>>>>>  508                 }
>>>>>>>  509             } catch (ClassCastException e) {
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 8/10/16, 1:59 AM, Prasanta Sadhukhan wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Please review a fix for an issue where it is seen that the 
>>>>>>>> selected printer tray is ignored in linux and default standard 
>>>>>>>> tray is used for printing.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6357887
>>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.00/
>>>>>>>>
>>>>>>>> Issue was lpr command needs "-o InputSlot=<trayname>" to select 
>>>>>>>> the printertray which was not being passed to lpr command.
>>>>>>>> Proposed fix added InputSlot option to lpr command to select 
>>>>>>>> the printertray.
>>>>>>>>
>>>>>>>> Tested in ubuntu and solaris11.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>
>>>>
>>>
>>