[OpenJDK 2D-Dev] [9] RFR: JDK-6529030, , Java Printing: Print range > Selection gets enabled

[prasanta sadhukhan]

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
>>>
>>