On Mon, 19 Sep 2022 23:24:45 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> On Windows, the insets obtained for a Non-Resizable AWT Frame was different >> when frame.pack() was called and subsequent call to frame.getInsets() or >> frame.getPreferredSize(). Due to this, the actual and preferred size >> differed when frame.pack() was called for Non-Resizable frame (on Windows). >> >> Earlier the insets returned when frame.getInsets() was called, was that of a >> Resizable frame and not the correct insets associated with Non-Resizable >> frame. Fix is added to native code to get the correct insets. The test - >> AwtFramePackTest.java has been updated to test actual and expected/preferred >> size for both Resizable and Non-Resizable Frames. >> >> The test is generic though the issue and fix is on Windows platform because >> the condition >> `frame.getSize() == frame.getPreferredSize()` should be true on all >> platforms when frame.pack() is called. >> >> Following is the link to Windows System Metrics (used for native insets) - >> https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getsystemmetrics > > Harshitha Onkar has updated the pull request incrementally with one > additional commit since the last revision: > > reverted the changes related to previous fix - checking the type of target > object Marked as reviewed by aivanov (Reviewer). src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 1411: > 1409: > 1410: if (m_insets.left < 0 || m_insets.top < 0 || > 1411: m_insets.right < 0 || m_insets.bottom < 0) { No strong opinion here, yet I'd rather leave these lines unchanged. Having the opening brace on its own line avoids the confusion between the condition and the code in the body; on the other hand, it starts with a comment which visually separates the condition from the code inside the if-block. src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 1419: > 1417: ::GetSystemMetrics(SM_CXSIZEFRAME) + > extraPaddedBorderInsets; > 1418: m_insets.top = m_insets.bottom = > 1419: ::GetSystemMetrics(SM_CYSIZEFRAME) + > extraPaddedBorderInsets; Is it worth adding a comment clarifying why the style isn't checked? Maybe not, since now it's gone from the code. On the other hand, a reader may wonder why we always uses the sizing frame insets even for non-resizeable frames. ------------- PR: https://git.openjdk.org/jdk/pull/9954
