[OpenJDK 2D-Dev] [9] RFR: JDK-6529030, , Java Printing: Print range > Selection gets enabled
prasanta sadhukhan
prasanta.sadhukhan at oracle.com
Wed May 11 10:45:59 UTC 2016
Gentle reminder for review.
Regards
Prasanta
On 4/21/2016 3:35 PM, 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
>>>>>>>
>>>>>>
>>>>
>>>
>>
>
More information about the 2d-dev
mailing list