[OpenJDK 2D-Dev] [9] RFR JDK-8165947: One more page printed before the test page with OpenJDK.
Philip Race
philip.race at oracle.com
Fri Sep 23 17:25:12 UTC 2016
+1
-phil.
On 9/23/16, 10:24 AM, Prasanta Sadhukhan wrote:
> Yes, sorry. Done
> http://cr.openjdk.java.net/~psadhukhan/8165947/webrev.03/
>
> Regards
> Prasanta
> On 9/23/2016 10:44 PM, Philip Race wrote:
>> 77 throw new RuntimeException("Banner page is
>> printed");
>>
>> that message needs to be conditionalized too.
>>
>> -phil
>>
>> On 9/23/16, 10:01 AM, Prasanta Sadhukhan wrote:
>>> Updated the testcase:
>>> http://cr.openjdk.java.net/~psadhukhan/8165947/webrev.02/
>>>
>>> Regards
>>> Prasanta
>>> On 9/23/2016 9:47 PM, Philip Race wrote:
>>>> The JDK changes are fine but I don't think the test is.
>>>> It assumes that the system default is no banner page.
>>>>
>>>> You need to update the test to check what the system default is
>>>> (banner or no banner) and have the tester "expect" whatever that
>>>> happens to be.
>>>>
>>>> -phil
>>>>
>>>> On 9/23/16, 8:30 AM, Prasanta Sadhukhan wrote:
>>>>> Hi Phil,
>>>>>
>>>>> Please find the modified webrev:
>>>>> http://cr.openjdk.java.net/~psadhukhan/8165947/webrev.01/
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 9/22/2016 11:10 PM, Prasanta Sadhukhan wrote:
>>>>>>
>>>>>>
>>>>>> On 9/22/2016 11:09 PM, Philip Race wrote:
>>>>>>> OK I see now.
>>>>>>> The comment should be updated to remove mention of the dialog
>>>>>>> and explain that instead.
>>>>>>>
>>>>>>> But in that case do you still need the "else" in setAttributes?
>>>>>>>
>>>>>> Yes, right. we do not need the else in setAttributes. will update
>>>>>> that and comment and publish new webrev.
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>>> -phil.
>>>>>>>
>>>>>>>
>>>>>>> On 9/22/16, 10:32 AM, Prasanta Sadhukhan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 9/22/2016 10:59 PM, Philip Race wrote:
>>>>>>>>> Your comment refers only (and explicitly) to the print dialog :-
>>>>>>>>> 1484 * printing Banner page through print dialog via setAttributes().
>>>>>>>>> So if we get into that code afterwards why do we need this new
>>>>>>>>> code ?
>>>>>>>>>
>>>>>>>> The new code is needed because if we have null attributes, then
>>>>>>>> setAttributes() will return
>>>>>>>> 1177 if (attributes == null || service == null) {
>>>>>>>> 1178 return;
>>>>>>>> 1179 }
>>>>>>>> even before it reaches
>>>>>>>>
>>>>>>>> 1271 JobSheets jobSheets =
>>>>>>>> (JobSheets)attributes.get(JobSheets.class);
>>>>>>>> 1272 if (jobSheets != null) {
>>>>>>>> 1273 noJobSheet = jobSheets == JobSheets.NONE;
>>>>>>>> 1274 } else {
>>>>>>>> 1275 JobSheets js = (JobSheets)getPrintService().
>>>>>>>> 1276 getDefaultAttributeValue(JobSheets.class);
>>>>>>>> 1277 if (js != null && js.equals(JobSheets.NONE)) {
>>>>>>>> 1278 noJobSheet = true;
>>>>>>>> 1279 }
>>>>>>>> 1280 }
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>>> I do see that call to setAttributes() but I am assuming it
>>>>>>>>> does not
>>>>>>>>> get in there, else why does it not work already for this case?
>>>>>>>>> It looks identical to your new code.
>>>>>>>>>
>>>>>>>>> Put another way I don't see how the bug even manifests if it
>>>>>>>>> works as you describe.
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>> On 9/22/16, 10:10 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>> That's why I call the new code before setAttributes() so that
>>>>>>>>>> user's selection is not overridden. I put a comment regarding
>>>>>>>>>> that in the fix.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Prasanta
>>>>>>>>>> On 9/22/2016 10:38 PM, Philip Race wrote:
>>>>>>>>>>> What happens if the application does not display a dialog but
>>>>>>>>>>> instead the application code explicitly does this:
>>>>>>>>>>>
>>>>>>>>>>> aset.add(JobSheets.STANDARD);
>>>>>>>>>>> print(aset)
>>>>>>>>>>>
>>>>>>>>>>> ?
>>>>>>>>>>>
>>>>>>>>>>> It appears to me you will over-ride that.
>>>>>>>>>>>
>>>>>>>>>>> -phil.
>>>>>>>>>>>
>>>>>>>>>>> On 9/22/16, 9:54 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>> Hi Phil,
>>>>>>>>>>>>
>>>>>>>>>>>> My new code takes care of the problem when attribute is not
>>>>>>>>>>>> set by the user who directly calls PrinterJob.print(). If
>>>>>>>>>>>> no print dialog is shown, then print(attr) seems to be
>>>>>>>>>>>> called with Null attribute
>>>>>>>>>>>> and since noJobSheet was false, it used to print the banner
>>>>>>>>>>>> page.
>>>>>>>>>>>> I now checked only the
>>>>>>>>>>>> PrintService.getDefaultAttributeValue(JobSheets.class) and
>>>>>>>>>>>> if IPP returns "none", I change the default noJobSheet to
>>>>>>>>>>>> true so that no banner page is printed (to honor system
>>>>>>>>>>>> default).
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>> Prasanta
>>>>>>>>>>>> On 9/22/2016 10:16 PM, Philip Race wrote:
>>>>>>>>>>>>> This looks wrong to me. Shouldn't the logic look like the
>>>>>>>>>>>>> one you have earlier in
>>>>>>>>>>>>> the file ? ie this :
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1271 JobSheets jobSheets =
>>>>>>>>>>>>> (JobSheets)attributes.get(JobSheets.class);
>>>>>>>>>>>>> 1272 if (jobSheets != null) {
>>>>>>>>>>>>> 1273 noJobSheet = jobSheets == JobSheets.NONE;
>>>>>>>>>>>>> 1274 } else {
>>>>>>>>>>>>> 1275 JobSheets js = (JobSheets)getPrintService().
>>>>>>>>>>>>> 1276 getDefaultAttributeValue(JobSheets.class);
>>>>>>>>>>>>> 1277 if (js != null &&
>>>>>>>>>>>>> js.equals(JobSheets.NONE)) {
>>>>>>>>>>>>> 1278 noJobSheet = true;
>>>>>>>>>>>>> 1279 }
>>>>>>>>>>>>> 1280 }
>>>>>>>>>>>>>
>>>>>>>>>>>>> As it is your new code seems to completely disregard any
>>>>>>>>>>>>> setting by
>>>>>>>>>>>>> the application in the attribute set - which *does not*
>>>>>>>>>>>>> have anything to do with
>>>>>>>>>>>>> whether a dialog was set.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Also I reject that this can be a TCK failure.
>>>>>>>>>>>>> "I got a banner page" is something that can be a system
>>>>>>>>>>>>> configuration
>>>>>>>>>>>>> parameter and is wholly outside anything JCK can (or
>>>>>>>>>>>>> should) care about.
>>>>>>>>>>>>> You could fix this but it could then behave the same on a
>>>>>>>>>>>>> different system.
>>>>>>>>>>>>> In fact my reading of the bug is simply that they noticed
>>>>>>>>>>>>> this when running
>>>>>>>>>>>>> a TCK test. That does not make it a TCK failure.
>>>>>>>>>>>>> I have removed the tck labels from the bug and JCK can
>>>>>>>>>>>>> argue with me if they want to ..
>>>>>>>>>>>>>
>>>>>>>>>>>>> -phil.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/16/16, 3:21 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review a fix for a tck failure in jdk9 whereby
>>>>>>>>>>>>>> "banner page" (cover page) is printed by default when
>>>>>>>>>>>>>> print() is called directly without any print dialog being
>>>>>>>>>>>>>> shown.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8165947
>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8165947/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Issue was in RasterPrinterJob, "noJobSheet" variable was
>>>>>>>>>>>>>> set to false which when passed to
>>>>>>>>>>>>>> PSPrinterJob#printExecCmd(), it results in adding "-o
>>>>>>>>>>>>>> job-sheets=standard" to lpr command and
>>>>>>>>>>>>>> therefore, Banner page was getting printed by default.
>>>>>>>>>>>>>> Proposed fix is to check for defaultAttributeValue for
>>>>>>>>>>>>>> JobSheets attribute so that we can find what is the
>>>>>>>>>>>>>> default value reported by underlying platform and set
>>>>>>>>>>>>>> "noJobSheet" value to default jobsheet native value
>>>>>>>>>>>>>> (like CUPS report job-sheet=none so that no banner page
>>>>>>>>>>>>>> is to be printed by default even though it supports
>>>>>>>>>>>>>> jobsheet)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I tested "6575247:Banner checkbox in PrinterJob print
>>>>>>>>>>>>>> dialog doesn't work" testcase in windows, solaris, linux
>>>>>>>>>>>>>> and it works as expected.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> JCK test
>>>>>>>>>>>>>> api/javax_swing/interactive/PrintTest.html#PrintTest via
>>>>>>>>>>>>>> command
>>>>>>>>>>>>>> "/jdk-9/bin/java -showversion
>>>>>>>>>>>>>> -Dswing.defaultlaf=com.sun.java.swing.plaf.motif.MotifLookAndFeel
>>>>>>>>>>>>>> -cp /root/jck/JCK-runtime-9/classes:
>>>>>>>>>>>>>> -Djava.security.policy=/root/jck/JCK-runtime-9/lib/jck.policy
>>>>>>>>>>>>>> javasoft.sqe.tests.api.java.awt.interactive.PrintTest
>>>>>>>>>>>>>> -platform.hasPrinter true -TestCaseID ALL"
>>>>>>>>>>>>>> also works ie no banner page is printed by default.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160923/6549a986/attachment.html>
More information about the 2d-dev
mailing list