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

Jennifer Godinez jennifer.godinez at oracle.com
Tue Apr 19 22:22:28 UTC 2016


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




More information about the 2d-dev mailing list