On Wed, 14 Feb 2024 09:08:59 GMT, Prasanta Sadhukhan <[email protected]> wrote:
> > For me, it fails on Windows because no suitable font is found; > > It is not because of fonts availability...It's because of the order of `pack > `and `setVisible `being called which is causing some issue with > `i18nFieldView `layouting. If the `pack` is called **before** `setVisible` > then it works in windows and other platforms, so one can see it's NOT because > of font availability It makes sense, actually: the square that's rendered instead of missing character takes some space, therefore the height shouldn't be zero. > Now, in one of the test-sprint, it was mentioned that _setLocationRelativeTo > followed by pack cause some issues_ so it is recommended to call `pack > `before `setLocationRelativeTo` [#13633 > (comment)](https://github.com/openjdk/jdk/pull/13633#discussion_r1178208629) Yes, I noticed it… in some cases, and it's somewhat to be expected: the size of the frame affects the location. > so in similar vein, I guess this can also argued that setVisible followed by > pack can cause some issue and be recommended that `pack` should be called > before `setVisible` I agree, you should rather call `pack` or `setSize` before calling `setVisible`… but you're not required to. There was a bug in `JScrollPane` which also reproduces only if `setVisible(true)` is called before `pack`. Here it is [JDK-4152524](https://bugs.openjdk.org/browse/JDK-4152524): ScrollPane AS_NEEDED always places scrollbars first time component is laid out. When the test for JDK-4152524 was open-sourced, `pack()` was added and the test stopped reproducing the original problem. See [PR 14478, `ScrollPaneExtraScrollBar.java`](https://github.com/openjdk/jdk/pull/14478/files#diff-dea8716fbb80746e76aa78045e9d9041794791704396d49d9ad9050786ce0fc5L60) The above means there's a workaround for the bug we're discussing, and the workaround follows the established best practices. > > And this test demonstrates it. The test is not to be removed. > > The fix and the number of regressions it caused (as mentioned in my previous > comment) demonstrated that the understanding was not correct so why should we > impose the test on the fixer of JDK-8001470..Let him/her devise a new > testcase for the fix he/she proposes (if we still think pack can be called > after setvisible but it can be argued why different thought process for order > of setLocationRelativeTo/pack and setVisible/pack) *The fix* caused regressions, *the fix* is backed out. *The test* is still relevant, in my opinion: it demonstrates the problem described in the bug. If and when a new fix for this bug is developed, the test can be used to verify the proposed fix. He or she will not need to write a new test (it's still needed to test the fix, isn't it?) and may still decide to modify the test or to write a new test if necessary. I see no reason to remove the test. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17528#issuecomment-1943532972
