[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-5049012: PrintToFile option is not disabled for flavors that do not
From: Philip Race <philip.race () oracle ! com>
Date: 2016-06-30 14:20:44
Message-ID: 57752ABC.4020406 () oracle ! com
[Download RAW message or body]
Eh ? Before your change the code was calling
isAttributeCategorySupported(), notisAttributeValueSupported()
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 ..
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 ?
-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/
>
> Regards
> Prasanta
> On 6/29/2016 11:59 PM, Philip Race wrote:
> > ---
> > https://docs.oracle.com/javase/8/docs/api/javax/print/PrintService.html#isAttribut \
> > eValueSupported-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/
> > >
> > > 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
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Eh ? Before your change the code was calling <br>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<pre><span class="removed">isAttributeCategorySupported(), not </span><span \
class="changed">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 ..
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 ?
-phil
</span></pre>
<br>
<br>
On 6/30/16, 12:45 AM, Prasanta Sadhukhan wrote:
<blockquote cite="mid:5774CE17.2090209@oracle.com" type="cite">Hi
Phil,
<br>
<br>
NPE will be thrown
<br>
----------
<br>
public boolean isAttributeValueSupported(Attribute attr,
<br>
\
DocFlavor flavor, <br>
\
AttributeSet attributes) {
<br>
if (attr == null) {
<br>
throw new NullPointerException("null attribute");
<br>
}
<br>
---------
<br>
before updateInfo() will be called so I guess null check is
redundant there. Anyways, I have added the check back just to be
safe
<br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.01/</a>
<br>
<br>
Regards
<br>
Prasanta
<br>
On 6/29/2016 11:59 PM, Philip Race wrote:
<br>
<blockquote type="cite">---
<br>
<a class="moz-txt-link-freetext" \
href="https://docs.oracle.com/javase/8/docs/api/javax/print/PrintService.html#isAttrib \
uteValueSupported-javax.print.attribute.Attribute-javax.print.DocFlavor-javax.print.at \
tribute.AttributeSet">https://docs.oracle.com/javase/8/docs/api/javax/print/PrintServi \
ce.html#isAttributeValueSupported-javax.print.attribute.Attribute-javax.print.DocFlavor-javax.print.attribute.AttributeSet</a>-
<br>
<br>
Throws:
<br>
NullPointerException - (unchecked exception) if attrval is
null.
<br>
<br>
-- <br>
<br>
So why did you remove the check for null ?
<br>
<br>
<br>
-phil.
<br>
<br>
On 6/28/16, 3:04 AM, Prasanta Sadhukhan wrote:
<br>
<blockquote type="cite">Hi All,
<br>
<br>
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
<br>
even though isAttributeValueSupported for that flavor returns
false.
<br>
<br>
Bug: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-5049012">https://bugs.openjdk.java.net/browse/JDK-5049012</a>
<br>
webrev:
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/5049012/webrev.00/</a>
<br>
<br>
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
<br>
and therefore the "Print-to-File" option gets enabled.
<br>
<br>
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.
<br>
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.
<br>
<br>
Regards
<br>
Prasanta
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic