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

[Vadim Pakhnushev]

You see, you now have
  102         MediaTray mt = null;
  114         return( mt);

This can be just return null;
No need for new webrev for that, +1.

Vadim

On 25.08.2016 19:14, Prasanta Sadhukhan wrote:
>
>
> On 8/25/2016 7:29 PM, Vadim Pakhnushev wrote:
>> In the PSPrinterJob.java customTray.getChoiceName() is not checked 
>> for null but in the UnixPrintJob it is checked.
>> Could this be a problem?
>>
> Checked for null just to be safe and simplified test method
> Modified webrev
> http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.03/
>
> Regards
> Prasanta
>> 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
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>