[OpenJDK 2D-Dev] [9] RFR: JDK-6529030, , Java Printing: Print range > Selection gets enabled
prasanta sadhukhan
prasanta.sadhukhan at oracle.com
Thu Apr 21 10:05:21 UTC 2016
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
>>>>>>
>>>>>
>>>
>>
>
More information about the 2d-dev
mailing list