<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Another gentle reminder, please review my code changes. Thank you in advance.<div class="">With Regards,</div><div class="">Avik Niyogi<br class=""><div><blockquote type="cite" class=""><div class="">On 14-Jul-2016, at 8:46 pm, Alexandr Scherbatiy <<a href="mailto:alexandr.scherbatiy@oracle.com" class="">alexandr.scherbatiy@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
The fix looks good to me.<br class="">
<br class="">
Thanks,<br class="">
Alexandr.<br class="">
<br class="">
<div class="moz-cite-prefix">On 7/14/2016 9:53 AM, Avik Niyogi
wrote:<br class="">
</div>
<blockquote cite="mid:45B95487-4728-464C-91B4-5B360955F963@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
Hi All,
<div class="">Please find my webrev below for review which
includes changes as perceived from inputs provided.</div>
<div class=""><br class="">
</div>
<div class=""><a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.02/" class="">http://cr.openjdk.java.net/~aniyogi/8160438/webrev.02/</a></div>
<div class=""><br class="">
</div>
<div class="">With Regards,</div>
<div class="">Avik Niyogi<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On 12-Jul-2016, at 7:12 pm, Alexandr
Scherbatiy <<a moz-do-not-send="true" href="mailto:alexandr.scherbatiy@oracle.com" class="">alexandr.scherbatiy@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class=""><span style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px; float: none; display:
inline !important;" class="">On 7/12/2016 8:24 AM, Avik
Niyogi wrote:</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">Hi All,<br class="">
A gentle reminder, please review my code changes.<br class="">
<br class="">
With Regards,<br class="">
Avik Niyogi<br class="">
<br class="">
<blockquote type="cite" class="">On 08-Jul-2016, at 4:24
pm, Yuri Nesterenko <<a moz-do-not-send="true" href="mailto:yuri.nesterenko@oracle.com" class="">yuri.nesterenko@oracle.com</a>>
wrote:<br class="">
<br class="">
It pass for me if I provide some time to process
native events<br class="">
after the user activity simulation. For instance,<br class="">
setAutoDelay(50) for robot does that plus
waitForIdle()<br class="">
after every mouseMove(). In this case, the part with
that additional<br class="">
check and a (misleading, I think) comment about the
color profiles<br class="">
may be removed. The test would take much more time
though, and<br class="">
@run main/timeout=600 bug8057791<br class="">
line would be necessary.<br class="">
</blockquote>
</blockquote>
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> Some
more comments to the previous ones:</span><br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> -
runColorTestCase() uses JList and should be run on EDT</span><br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> -
"if (tryNimbusLookAndFeel()) {" It is supposed that the
Nimbus L&F is supported on all platforms. May be it
is better to fail the test if the Nimbus L&F is not
set.</span><br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> -
"if (osName.contains("Mac")) { isMac = true; }" can be
simplified to "return osName.contains("Mac")"</span><br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> - "
if (!"".equals(errorString)) {" can be simplified to
!errorString.isEmpty()</span><br style="font-family:
Helvetica; font-size: 12px; font-style: normal;
font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: normal; orphans:
auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> Thanks,</span><br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class=""> Alexandr.</span><br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" class=""><br class="">
Thanks,<br class="">
-yan<br class="">
<br class="">
On 07/08/2016 04:28 AM, Avik Niyogi wrote:<br class="">
<blockquote type="cite" class="">The test does not
pass if mac specific check is not done for<br class="">
backgroundcolor.<br class="">
The check is required to get the same values from
BufferedImage as they<br class="">
are the same as found with Digital Color Meter.<br class="">
This test case fixes that.<br class="">
Please provide inputs if this fix will get a +1 or
if not any positive<br class="">
inputs to modify the test case will be welcome.<br class="">
<br class="">
With Regards,<br class="">
Avik Niyogi<br class="">
<blockquote type="cite" class="">On 07-Jul-2016, at
9:51 pm, Semyon Sadetsky<br class="">
<<a moz-do-not-send="true" href="mailto:semyon.sadetsky@oracle.com" class="">semyon.sadetsky@oracle.com</a> <<a moz-do-not-send="true" href="mailto:semyon.sadetsky@oracle.com" class="">mailto:semyon.sadetsky@oracle.com</a>>>
wrote:<br class="">
<br class="">
<br class="">
<br class="">
On 7/7/2016 6:30 PM, Yuri Nesterenko wrote:<br class="">
<blockquote type="cite" class="">On 07/07/2016
06:15 PM, Semyon Sadetsky wrote:<br class="">
<blockquote type="cite" class=""><br class="">
On 7/7/2016 5:58 PM, Yuri Nesterenko wrote:<br class="">
<blockquote type="cite" class="">On 07/07/2016
05:35 PM, Yuri Nesterenko wrote:<br class="">
<blockquote type="cite" class="">On
07/07/2016 05:04 PM, Semyon Sadetsky
wrote:<br class="">
<blockquote type="cite" class=""><br class="">
On 07.07.2016 16:35, Avik Niyogi wrote:<br class="">
<blockquote type="cite" class="">Hi
Semyon,<br class="">
<br class="">
Thank you for the review comment.<br class="">
<br class="">
In Mac OS X, *System Preferences >
Displays > Colors > Display<br class="">
Profile* section, the default value
is *Color LCD*.<br class="">
This causes a failure in some test
cases which uses robot.The colour<br class="">
configuration it expects to use is the
*Generic RGB Profile*.<br class="">
That is what “Non-generic display
settings” means.<br class="">
</blockquote>
AFAIK there are instruction that tests
should be executed using color<br class="">
profile with no color corrections on OS
X. I guess there are many<br class="">
other<br class="">
tests that fail with color correction.<br class="">
If this is a root cause than the bug
doesn't need to be fixed.<br class="">
</blockquote>
Well, I filed this bug and I used Generic
RGB on all my<br class="">
test machines. There may be additional
precautions in the tests<br class="">
about that but misconfiguration is not the
root case here.<br class="">
That said, I feel vary about attempts to
guess Apple colors<br class="">
</blockquote>
wary I mean<br class="">
<blockquote type="cite" class="">in
non-generic profiles.<br class="">
</blockquote>
</blockquote>
Yuri. Do you mean that the non-generic is not
a root case?<br class="">
</blockquote>
No: I had Generic RGB everywhere, and it failed
for me.<br class="">
I should say that the new version of the test
properly<br class="">
fails with b120 and pass with current PIT. And
colors<br class="">
are visibly different in the two builds: so the
test works OK now.<br class="">
</blockquote>
That contradicts with the description of the fix.<br class="">
Probably the test does unnecessary care about the
non-Generic profiles.<br class="">
<br class="">
159 if (!foundMatch &&
isMac()) {<br class="">
160 //One more chance for Mac
due to non-Generic<br class="">
display setting<br class="">
161 detectedColor = new
Color(img.getRGB(x, y), true);<br class="">
162 if
(detectedColor.equals(colorCheck)) {<br class="">
163 foundMatch = true;<br class="">
164 break checkLoops;<br class="">
165 }<br class="">
166 }<br class="">
<br class="">
Does it mean that the code above, which is
necessary to serve<br class="">
non-Generic profiles on OS X, may be removed and
the test still passes?<br class="">
<br class="">
--Semyon<br class="">
<blockquote type="cite" class="">-yan<br class="">
<br class="">
<blockquote type="cite" class="">--Semyon<br class="">
<blockquote type="cite" class="">
<blockquote type="cite" class="">-yan<br class="">
<br class="">
<br class="">
<blockquote type="cite" class="">--Semyon<br class="">
<blockquote type="cite" class="">Regarding
“Negative scenarios”, these include
cases where<br class="">
colours are<br class="">
different from the expected background
or foreground colors<br class="">
is checked with the robot and
BufferedImage respectively to<br class="">
*eliminate<br class="">
false positives due to coincidentally
finding a match* by using<br class="">
robot<br class="">
or BufferedImage.<br class="">
<br class="">
Please find my changes appropriating
the inputs provided.<br class="">
I removed the variable foundMatch as I
found that it is not required<br class="">
at all and incorporated the suggestion
to use return instead of a<br class="">
labelled break.<br class="">
<br class="">
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.01/" class="">http://cr.openjdk.java.net/~aniyogi/8160438/webrev.01/</a><br class="">
<a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.01/"><http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.01/></a><br class="">
<br class="">
<br class="">
<blockquote type="cite" class="">On
07-Jul-2016, at 6:30 pm, Semyon
Sadetsky<br class="">
<<a class="moz-txt-link-abbreviated" href="mailto:semyon.sadetsky@oracle.com">semyon.sadetsky@oracle.com</a>
<a class="moz-txt-link-rfc2396E" href="mailto:semyon.sadetsky@oracle.com"><mailto:semyon.sadetsky@oracle.com></a>><br class="">
wrote:<br class="">
<br class="">
Hi Avik,<br class="">
<br class="">
could you clarify what is
"Non-generic display settings"? Is
it<br class="">
known<br class="">
bug on OS X?<br class="">
And also please be more specific on
"negative scenarios" why<br class="">
they are<br class="">
necessary ?<br class="">
<br class="">
Also could you replace labeled break
with "return foundMatch; "<br class="">
<br class="">
--Semyon<br class="">
<br class="">
<br class="">
On 07.07.2016 15:11, Avik Niyogi
wrote:<br class="">
<blockquote type="cite" class="">Hi
All,<br class="">
<br class="">
Kindly review the fix for JDK9.<br class="">
<br class="">
*Bug:<br class="">
*<a class="moz-txt-link-rfc2396E" href="https://bugs.openjdk.java.net/browse/JDK-8160438"><https://bugs.openjdk.java.net/browse/JDK-8160438></a><a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8160438">https://bugs.openjdk.java.net/browse/JDK-8160438</a><br class="">
<br class="">
<br class="">
<br class="">
*Webrev:<br class="">
*<a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.00/"><http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.00/></a><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~aniyogi/8160438/webrev.00/">http://cr.openjdk.java.net/~aniyogi/8160438/webrev.00/</a><br class="">
<br class="">
<br class="">
<br class="">
*Issue: *test
javax/swing/plaf/nimbus/8057791/bug8057791.java<br class="">
consistently fails on OS X 10.10<br class="">
<br class="">
*Cause: * Due to bug in detecting
color for Non-generic display<br class="">
settings for Mac hardware, test
case failed<br class="">
<br class="">
*Fix: *Positive and negative
scenarios was added to check the<br class="">
colour<br class="">
for the Nimbus LAF foreground and
background colours.<br class="">
<br class="">
With Regards,<br class="">
Avik Niyogi</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></div></body></html>