[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR: JDK-6529030, , Java Printing: Print range > Selection gets enabled
From:       Phil Race <philip.race () oracle ! com>
Date:       2016-05-13 21:24:51
Message-ID: 57364623.7060403 () oracle ! com
[Download RAW message or body]

+1

-phil

On 04/21/2016 03:05 AM, prasanta sadhukhan wrote:
> Hi Phil,
> 
> As we discussed that java.awt.PrintJob should have selection radio 
> button enabled as JobAttributes supports selection
> but java.awt.print.PrinterJob should NOT have selection radio button 
> enabled as at API level, we do not support selection.
> 
> So, as per suggestion by Jennifer I have modified the 
> setNativeAttributes to check for PD_NOSELECTION and found we conform 
> to the above theory.
> We have selection button enabled for jdk1.1 printjob but disabled for 
> jdk1.2 PrinterJob.
> 
> Also, since it is a windows specific code fix, it will not effect any 
> other platform.
> Please review modified webrev:
> http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.02/
> 
> Regards
> Prasanta
> On 4/20/2016 2:39 PM, prasanta sadhukhan wrote:
> > Hi Jennifer,
> > 
> > If I do not set SunPageSelection attribute in setNativeAttributes() 
> > like this
> > 
> > + if ((flags & PD_NOSELECTION) != PD_NOSELECTION) {
> > if ((flags & PD_PAGENUMS) != 0) {
> > attributes.add(SunPageSelection.RANGE);
> > } else if ((flags & PD_SELECTION) != 0) {
> > attributes.add(SunPageSelection.SELECTION);
> > } else {
> > attributes.add(SunPageSelection.ALL);
> > }
> > +     }
> > without doing my fix in awt_PrintControl.cpp#InitPrintDialog() it 
> > will disable the selection radio button for both invocation as 
> > getSelectAttrib() returns PD_NOSELECTION.
> > I think my fix in InitPrintDialog() 
> > http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.01/
> > is required for this fix.
> > 
> > Regards
> > Prasanta
> > On 4/20/2016 3:52 AM, Jennifer Godinez wrote:
> > > Hi Prasanta,
> > > 
> > > It looks to me that we missed to check (flags & PD_NOSELECTION) in 
> > > setNativeAttributes that's why we are setting SunPageSelection 
> > > attribute when we shouldn't.  I think that is where we should put 
> > > the fix.
> > > 
> > > Jennifer
> > > 
> > > On 04/15/2016 03:34 AM, prasanta sadhukhan wrote:
> > > > Hi Phil,
> > > > 
> > > > On 4/13/2016 10:40 PM, Philip Race wrote:
> > > > > This seems reasonable to me - we don't want "disable" the 
> > > > > selection radio button
> > > > > and prevent the user from selecting it since our API does support 
> > > > > it as an option.
> > > > > The way I first read the bug report was that it implied that the 
> > > > > first invocation
> > > > > when the selection button was disabled was "right" and the 2nd 
> > > > > invocation
> > > > > was "wrong" whereas it seems the other way around.
> > > > > 
> > > > > If the updated code (and my understanding) is correct then should we
> > > > > not in fact be updating getSelectAttrib() so that it never returns 
> > > > > PD_NOSELECTION,
> > > > > rather than "fixing it up" in this code ?
> > > > > 
> > > > > Of course you then need to look to see how we interpret & use a
> > > > > return value of PD_NOSELECTIONfrom getSelectAttrib() on OS X and 
> > > > > Linux.
> > > > > 
> > > > getSelectAttrib() is not called in linux.
> > > > But it's called in osx and we determine to/from pages to determine 
> > > > which radio button (All/Pages) to select
> > > > http://hg.openjdk.java.net/jdk9/client/jdk/file/735a130dc8db/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m#l388 \
> > > >  
> > > > 
> > > > Regards
> > > > Prasanta
> > > > > I would really like to get Jennifer's input on this.
> > > > > 
> > > > > -phil.
> > > > > 
> > > > > On 4/13/16, 4:17 AM, prasanta sadhukhan wrote:
> > > > > > Hi Phil,
> > > > > > 
> > > > > > On 4/13/2016 4:52 AM, Phil Race wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > My reading  here :
> > > > > > > https://msdn.microsoft.com/en-us/library/windows/desktop/ms646843%28v=vs.85%29.aspx \
> > > > > > >  
> > > > > > > 
> > > > > > > of the meaning of PD_NOSELECTION is that it disables the 
> > > > > > > SELECTION radio button such
> > > > > > > that the user *cannot* select it.
> > > > > > > 
> > > > > > > Is that true ? 
> > > > > > Yes, PD_NOSELECTION will disable SELECTION radio button.
> > > > > > > If so this seems like it cannot be the right fix for this issue
> > > > > > > and I am not sure why getSelectAttrib() would want to return that.
> > > > > > protected final int getSelectAttrib() {
> > > > > > if (attributes != null) {
> > > > > > SunPageSelection pages =
> > > > > > (SunPageSelection)attributes.get(SunPageSelection.class);
> > > > > > if (pages == SunPageSelection.RANGE) {
> > > > > > return PD_PAGENUMS;
> > > > > > } else if (pages == SunPageSelection.SELECTION) {
> > > > > > return PD_SELECTION;
> > > > > > } else if (pages ==  SunPageSelection.ALL) {
> > > > > > return PD_ALLPAGES;
> > > > > > }
> > > > > > }
> > > > > > return PD_NOSELECTION;
> > > > > > }
> > > > > > so if SunPageSelection is not added to attribute which was the 
> > > > > > case in 1st instance so PD_NOSELECTION is returned
> > > > > > and we have in awt_PrintControl.cpp#InitPrintDialog()
> > > > > > if (selectType != 0) { selectType will be 4 for PD_NOSELECTION 
> > > > > > so pd.Flags will be set and Selection radio button is disabled in 
> > > > > > 1st instance
> > > > > > pd.Flags |= selectType;
> > > > > > }
> > > > > > > Perhaps we never in practice return PD_NOSELECTION ?
> > > > > > > 
> > > > > > > I am also unsure what it means to set flags to PD_NOSELECTION | 
> > > > > > > PD_SELECTION
> > > > > > > as the windows docs don't tell me enough.
> > > > > > It will still disable SELECTION radio button.
> > > > > > > 
> > > > > > > Maybe what we want is just that we do not cause PD_SELECTION to 
> > > > > > > be set
> > > > > > > rather than setting PD_NOSELECTION.
> > > > > > > 
> > > > > > I have modified the webrev to not set PD_NOSELECTION if 
> > > > > > getSelectAttrib() returns NOSELECTION so it will mean Selection 
> > > > > > radiobutton will be enabled now but will not be selected UNLESS 
> > > > > > user selects
> > > > > > job.setDefaultSelection(JobAttributes.DefaultSelectionType.SELECTION) 
> > > > > > explicitly in the application.
> > > > > > 
> > > > > > http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.01/
> > > > > > Also, I added @requires (os.family == windows) tag to make sure 
> > > > > > the test is run on windows only as in linux, osx we do not get 
> > > > > > the "selection" option [only All and Page range] for this test.
> > > > > > 
> > > > > > Regards
> > > > > > Prasanta
> > > > > > > But to decide what to do I need to know the real effect of 
> > > > > > > PD_NOSELECTION.
> > > > > > > 
> > > > > > > BTW about the test: you should make sure that the instructions are
> > > > > > > valid on OS X and Linux ..
> > > > > > > 
> > > > > > > -phil.
> > > > > > > 
> > > > > > > On 04/06/2016 03:09 AM, prasanta sadhukhan wrote:
> > > > > > > > Hi All,
> > > > > > > > 
> > > > > > > > Please review a fix for jdk9.
> > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-6529030
> > > > > > > > webrev: http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.00/
> > > > > > > > 
> > > > > > > > The issue was when using java.awt.print.PrinterJob instance 
> > > > > > > > more then once, Selection radio button in Print dialog gets 
> > > > > > > > enabled from 2nd instance even though we are printing "All" pages
> > > > > > > > however Selection radio button is disabled in the first instance.
> > > > > > > > This is seen in windows.
> > > > > > > > 
> > > > > > > > This is because initially when windows initialized the print 
> > > > > > > > dialog, it calls InitPrintDialog() which calls 
> > > > > > > > getSelectAttrib() to find out which selection attribute user 
> > > > > > > > has selected.
> > > > > > > > getSelectAttrib() returns PD_NOSELECTION as 
> > > > > > > > SunPageSelection.class was not added to attribute.
> > > > > > > > So, in InitPrintDialog() we set PRINTDLG flags to 
> > > > > > > > PD_NOSELECTION. So, we see "Selection " radio button is 
> > > > > > > > disabled in 1st instance of print dialog.
> > > > > > > > Now, when user presses "OK", windows native code calls 
> > > > > > > > setNativeAttributes() and adds SunPageSelection.class to the 
> > > > > > > > attribute with SunPageSelection.ALL or SunPageSelection.RANGE.
> > > > > > > > 
> > > > > > > > When 2nd print dialog is shown, InitPrintDialog() will again 
> > > > > > > > call getSelectAttrib() which will now return 
> > > > > > > > SunPageSelection.ALL/SunPageSelection.RANGE which will be set 
> > > > > > > > to PRINTDLG flag but
> > > > > > > > we are not disabling Selection radio button, so in 2nd 
> > > > > > > > instance, Selection radio button gets enabled.
> > > > > > > > 
> > > > > > > > Fix was to check if PD_SELECTION attribute is selected by user, 
> > > > > > > > if not , sets PRINTDLG flag with PD_NOSELECTION.
> > > > > > > > 
> > > > > > > > I have checked 8151590 and 8061267 works correctly with this 
> > > > > > > > patch.
> > > > > > > > 
> > > > > > > > Regards
> > > > > > > > Prasanta
> > > > > > > 
> > > > > > 
> > > > 
> > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic