[OpenJDK 2D-Dev] [9] RFR JDK-5049012: PrintToFile option is not disabled for flavors that do not support destination

[Prasanta Sadhukhan]

Hi Jay,

Thanks for pointing that out.
Modified testcase to run in linux only and added check to fail if no 
printer service is found.
http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.03/

Regards
Prasanta
On 7/8/2016 4:00 PM, Jayathirth D V wrote:
>
> Hi Prasanta,
>
> Since CUPS is not used in Windows, your change will not make 
> difference if I run the test in Windows.
>
> Please add @requires tag in jtreg test case and there are some lines 
> which are 80 character plus.
>
> Also please add null check when you create PrinterService in test case.
>
> Thanks,
>
> Jay
>
> *From:*Philip Race
> *Sent:* Friday, July 08, 2016 4:46 AM
> *To:* Prasanta Sadhukhan
> *Cc:* 2d-dev
> *Subject:* Re: [OpenJDK 2D-Dev] [9] RFR JDK-5049012: PrintToFile 
> option is not disabled for flavors that do not support destination
>
> Ok .. +1
>
> -phil.
>
> On 7/1/16, 2:23 AM, Prasanta Sadhukhan wrote:
>
>     Hi Phil,
>
>     On 6/30/2016 7:50 PM, Philip Race wrote:
>
>         Eh ? Before your change the code was calling
>
>         isAttributeCategorySupported(), not isAttributeValueSupported()
>
>         So there was not previously a possibility of that NPE, and
>
>         my point was now you have changed that call you need that
>         !=null check
>
>         even more ..
>
>     Yes, I got that, right immediately after sending the below mail
>     and sent another email clarifying my misunderstanding.
>
>         But I think there is still a problem. Now dstSupported is set
>         only if
>
>         the *specific* destination is supported.
>
>         A malformed file: URL would not be valid but should not
>
>         disable (grey out) the print to file checkbox which
>
>         is what it looks like here .. can you verify this ?
>
>     Yes, we will grey out the checkbox for malformed url (like
>     files::/tmp/output.ps) as I found out. Please find modified webrev
>     that takes care of this problem too:
>     http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.02/
>     <http://cr.openjdk.java.net/%7Epsadhukhan/5049012/webrev.02/>
>
>     Regards
>     Prasanta
>
>         -phil
>
>
>
>         On 6/30/16, 12:45 AM, Prasanta Sadhukhan wrote:
>
>             Hi Phil,
>
>             NPE will be thrown
>             ----------
>             public boolean isAttributeValueSupported(Attribute attr,
>                                                          DocFlavor
>             flavor,
>             AttributeSet attributes) {
>                     if (attr == null) {
>                         throw new NullPointerException("null attribute");
>                     }
>             ---------
>             before updateInfo() will be called so I guess null check
>             is redundant there. Anyways, I have added the check back
>             just to be safe
>             http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.01/
>             <http://cr.openjdk.java.net/%7Epsadhukhan/5049012/webrev.01/>
>
>             Regards
>             Prasanta
>             On 6/29/2016 11:59 PM, Philip Race wrote:
>
>                 ---
>                 https://docs.oracle.com/javase/8/docs/api/javax/print/PrintService.html#isAttributeValueSupported-javax.print.attribute.Attribute-javax.print.DocFlavor-javax.print.attribute.AttributeSet-
>
>
>                 Throws:
>                     NullPointerException - (unchecked exception) if
>                 attrval is null.
>
>                 -- 
>
>                 So why did you remove the check for null ?
>
>
>                 -phil.
>
>                 On 6/28/16, 3:04 AM, Prasanta Sadhukhan wrote:
>
>                     Hi All,
>
>                     Please review a fix for an issue where it is seen
>                     that "Print-To-File" option is enabled even for
>                     flavors that do not support Destination attribute
>                     even though isAttributeValueSupported for that
>                     flavor returns false.
>
>                     Bug: https://bugs.openjdk.java.net/browse/JDK-5049012
>                     webrev:
>                     http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.00/
>                     <http://cr.openjdk.java.net/%7Epsadhukhan/5049012/webrev.00/>
>
>
>                     The reason for this behaviour is in Print Dialog
>                     code, the validation is done against
>                     isAttributeCategorySupported() which still returns
>                     true because Pageable and Printable supports it
>                     and therefore the "Print-to-File" option gets
>                     enabled.
>
>                     But print dialog is shown specific to a doc flavor
>                     and a print service (selected one) and hence must
>                     validate the attribute against the chosen doc flavor.
>                     So, the proposed fix checks the attributevalue is
>                     supported or not for that flavor to determine if
>                     we need to enable "Print-to-File" option in print
>                     dialog.
>
>                     Regards
>                     Prasanta
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160708/d574dddc/attachment.html>