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
