<Swing Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

<Swing Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

Sergey Bylokhov
Hello,
Please review the fix for jdk9.
In the fixes for JDK-7072653 [1] and JDK-8129838 [2] and JDK-8144161[3]


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.


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.


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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

Alexandr Scherbatiy

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]


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.


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.


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.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

Alexander Zvegintsev

+1

Thanks,
Alexander.
On 15/03/2017 15:33, Alexandr Scherbatiy wrote:

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]


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.


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.


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.



Loading...