The fix looks good to me.

Thanks,
Alexandr.

On 3/14/2017 6:12 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk9.
In the fixes for JDK-7072653 [1] and JDK-8129838 [2] and JDK-8144161[3]

[1] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/7606d0af7b80

Notes about JDK-7072653
- AquaComboBoxPopup.java the code "if (py + ph > scrBounds.y + scrBounds.height)» compares the vars in a different coordinate spaces(combobox vs screen). It is ok in BasicComboPopup.java because we translate scrBounds properly. - In Aqua the code was changed for the case when "JComboBox.isPopDown» property is false, I will create a separate CR to fix it when the property is true.

[2] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/5ea7cbf6a7bd

Notes about JDK-8129838
- AquaComboBoxPopup.java the second loop was changed to use "intersects" and "contains" at the same time, but if the point is located on the screen it will be found in the first loop, but if it was not found the second loop will be noop. - Some code was extracted to the new method getAvailableScreenArea(), but it was copied from the place where the only one screen was available, so it uses «0» as a left point and does not take into account that the screen can have non-zero offset.

[3] hg.openjdk.java.net/jdk9/jdk9/jdk/rev/e8e9c65def6d <http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/e8e9c65def6d>

Notes about JDK-8144161:
- After JDK-7072653 [1] and JDK-8129838 were pushed the test which was pushed started to fail on OS X, and this was considered as a testbug, even if the code in Aqua l&f was changed to support this functionality. - The test was changed to use a different look and feels, but since it run the test code a few times it start to work incorrectly, because JFrame.getWindows() can return popup window from the different L&F.(dispose was called on it but it was not garbage collected).

Fix description:
- An additional check in the second loop in AquaComboBoxPopup.java was removed. - AquaComboBoxPopup.getAvailableScreenArea now takes into account that we can have a few screens. - AquaComboBoxPopup.computePopupBounds() was changed to take into account that scrBounds is in the screen coordinates space.
  - The test bug7072653.java now check all screen in the system.
- Note that the new test ChoicePopupLocation.java will fail on linux in multiscreen because of JDK-8160270. I will fix it after this one.


Bug: https://bugs.openjdk.java.net/browse/JDK-8176448
Webrev can be found at: http://cr.openjdk.java.net/~serb/8176448/webrev.04/ <http://cr.openjdk.java.net/%7Eserb/8176448/webrev.04/>

Reply via email to