[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8061258, , [macosx] PrinterJob's native Print Dialog does not refl
From: Jayathirth D V <jayathirth.d.v () oracle ! com>
Date: 2016-03-30 10:20:47
Message-ID: a3f208ba-7c03-4c8b-a107-ecbb87c5224a () default
[Download RAW message or body]
Hi Prasanta,
Changes are fine and test logic works fine.
In test case, There are some trailing white spaces, lines with more than 80 \
character, and typo's in println please modify them before check-in.
Also bug mentions that that both no.of copies and from/to page are not getting set. \
As we discussed it looks like only PageRanges is not working.
You can update the bug regarding the same with Fix version as 9.
Thanks,
Jay
From: prasanta sadhukhan
Sent: Wednesday, March 30, 2016 12:18 PM
To: Phil Race; Jayathirth D V
Cc: 2d-dev@openjdk.java.net
Subject: Re: [9] RFR: JDK-8061258,,[macosx] PrinterJob's native Print Dialog does not \
reflect specified Copies or Page Ranges
Hi Phil,
I have updated the webrev with your proposed change.
http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/
Jay, can you please review and give your +1 on this?
Regards
Prasanta
On 3/29/2016 11:17 PM, Phil Race wrote:
Hi,
You are calling these unconditionally :
387 jint minPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMinPage);
388 jint maxPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMaxPage);
But it seems they are only used/needed in the else {..}
Other than that it looks reasonable but I have not had time to
apply the patch and verify it ..
-phil.
On 03/23/2016 11:47 PM, prasanta sadhukhan wrote:
Hi Phil,
On 3/24/2016 4:39 AM, Philip Race wrote:
On 3/23/16, 4:02 AM, prasanta sadhukhan wrote:
Hi Phil,
Please review a fix for jdk9
Bug: https://bugs.openjdk.java.net/browse/JDK-8061258
Gosh, it appears I submitted that bug 18 months ago although
I don't remember how I came across the problem.
I need to understand the fix better and it doesn't help that I don't understand this \
line :-
} else if (selectID == 3) {
Yes, this is wrong. I was thinking selectID ==3 was for PD_SELECTION but it was not.
Anyways, I have rectified this and updated the webrev:
HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.01/"http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.01/
where the else part handles both
PD_SELECTION or PD_NOSELECTION
I tested with JobAttributes.setDefaultSelection(ALL/RANGE/SELECTION) and also without \
defaultselection and all are working fine.
Regards
Prasanta
SelectID is set from a single flag bit .. so it will never be 3,
at least not anywhere I can find. What case is that supposed to
be covering ?
-phil.
webrev :HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.00/"http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.00/ \
When user attempts to pre-populate the native dialog with copies and page ranges by \
calling PrintRequestAttributeSet aset = new HashPrintRequestAttributeSet();
aset.add(new Copies(2));
aset.add(new PageRanges(3,4));
the print dialog does not reflect the user-defined setting.
This is because osx native code was calling getNumberOfPages() from OpenBook.java \
which was returning UNKNOWN_NUMBER_OF_PAGES (-1). Since getNumberOfPages() returned \
-1, osx always selected NSPrintAllPages or All Radio button.
I fixed it by removing this call to getNumberOfPages() and rely on \
getMinPage/getMaxPage() as was done in windows \
(awt_PrintControl.cpp:AwtPrintControl::InitPrintDialog) to select which radio button \
to be selected. If fromPage > minPage or toPage < maxPage, it means user has \
selected page ranges so PageRange radio button is to be selected else "All" radio \
button to be selected.
Regards
Prasanta
[Attachment #3 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type \
content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 \
(filtered medium)"><style><!-- /* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.new
{mso-style-name:new;}
span.changed
{mso-style-name:changed;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body bgcolor=white lang=EN-US link=blue \
vlink=purple><div class=WordSection1><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Hi \
Prasanta,<o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Changes are \
fine and test logic works fine. <o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>In test case, \
There are some trailing white spaces, lines with more than 80 character, and typo's \
in println please modify them before check-in.<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Also bug \
mentions that that both no.of copies and from/to page are not getting set. As we \
discussed it looks like only PageRanges is not working.<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>You can \
update the bug regarding the same with Fix version as 9.<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Thanks,<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Jay<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><div><div \
style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p \
class=MsoNormal><b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'>From:</span></b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'> prasanta \
sadhukhan <br><b>Sent:</b> Wednesday, March 30, 2016 12:18 PM<br><b>To:</b> Phil \
Race; Jayathirth D V<br><b>Cc:</b> 2d-dev@openjdk.java.net<br><b>Subject:</b> Re: [9] \
RFR: JDK-8061258,,[macosx] PrinterJob's native Print Dialog does not reflect \
specified Copies or Page Ranges<o:p></o:p></span></p></div></div><p \
class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Hi Phil,<br><br>I have \
updated the webrev with your proposed change.<br><a \
href="http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/</a><br><br>Jay, \
can you please review and give your +1 on \
this?<br><br>Regards<br>Prasanta<o:p></o:p></p><div><p class=MsoNormal>On 3/29/2016 \
11:17 PM, Phil Race wrote:<o:p></o:p></p></div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><div><pre>Hi,<o:p></o:p></pre><pre><o:p> </o:p></pre><pre>You \
are calling these unconditionally :<o:p></o:p></pre><pre><span class=new> 387 \
jint minPage = JNFCallIntMethod(env, srcPrinterJob, \
jm_getMinPage);</span><o:p></o:p></pre><pre><span class=new> 388 jint maxPage \
= JNFCallIntMethod(env, srcPrinterJob, jm_getMaxPage);</span><o:p></o:p></pre><p \
class=MsoNormal><br>But it seems they are only used/needed in the else \
{..}<br><br>Other than that it looks reasonable but I have not had time to<br>apply \
the patch and verify it ..<br><br>-phil.<br><br>On 03/23/2016 11:47 PM, prasanta \
sadhukhan wrote:<o:p></o:p></p></div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal \
style='margin-bottom:12.0pt'>Hi Phil,<o:p></o:p></p><div><p class=MsoNormal>On \
3/24/2016 4:39 AM, Philip Race wrote:<o:p></o:p></p></div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><br><br>On 3/23/16, \
4:02 AM, prasanta sadhukhan wrote: <o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>Hi Phil, \
<br><br>Please review a fix for jdk9 <br>Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8061258">https://bugs.openjdk.java.net/browse/JDK-8061258</a> \
<o:p></o:p></p></blockquote><p class=MsoNormal><br>Gosh, it appears I submitted that \
bug 18 months ago although<br>I don't remember how I came across the \
problem.<br><br>I need to understand the fix better and it doesn't help that I don't \
understand this line :-<o:p></o:p></p><pre><span class=changed> } else if (selectID \
== 3) {<o:p></o:p></span></pre></blockquote><p class=MsoNormal>Yes, this is wrong. I \
was thinking selectID ==3 was for PD_SELECTION but it was not.<br>Anyways, I have \
rectified this and updated the webrev:<br><a \
href="http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.01/</a><br><br>where \
the else part handles both <o:p></o:p></p><pre><span class=new>PD_SELECTION or \
PD_NOSELECTION<o:p></o:p></span></pre><pre><span \
class=new><o:p> </o:p></span></pre><pre><span class=new>I tested with \
JobAttributes.setDefaultSelection(ALL/RANGE/SELECTION) and also without \
defaultselection and all are working fine.<o:p></o:p></span></pre><p \
class=MsoNormal>Regards<br>Prasanta<br><br><o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><pre><span \
class=changed><o:p> </o:p></span></pre><pre><span class=changed>SelectID is set \
from a single flag bit .. so it will never be 3,<o:p></o:p></span></pre><pre><span \
class=changed>at least not anywhere I can find. What case is that supposed \
to<o:p></o:p></span></pre><pre><span class=changed>be covering \
?<o:p></o:p></span></pre><pre><span \
class=changed><o:p> </o:p></span></pre><pre><span \
class=changed>-phil.<o:p></o:p></span></pre><p \
class=MsoNormal><br><br><o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>webrev :<a \
href="http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.00/</a> \
<br><br>When user attempts to pre-populate the native dialog with copies and page \
ranges by calling <br> PrintRequestAttributeSet aset = new \
HashPrintRequestAttributeSet(); <br> \
aset.add(new Copies(2)); <br> aset.add(new \
PageRanges(3,4)); <br>the print dialog does not reflect the user-defined setting. \
<br><br>This is because osx native code was calling getNumberOfPages() from \
OpenBook.java which was returning UNKNOWN_NUMBER_OF_PAGES (-1). <br>Since \
getNumberOfPages() returned -1, osx always selected NSPrintAllPages or All Radio \
button. <br><br>I fixed it by removing this call to getNumberOfPages() and rely on \
getMinPage/getMaxPage() as was done in windows \
(awt_PrintControl.cpp:AwtPrintControl::InitPrintDialog) to select which radio button \
to be selected. <br>If fromPage > minPage or toPage < maxPage, it means user \
has selected page ranges so PageRange radio button is to be selected else \
"All" radio button to be selected. <br><br>Regards <br>Prasanta \
<o:p></o:p></p></blockquote></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote><p \
class=MsoNormal><o:p> </o:p></p></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic