On Tue, 3 Jun 2025 05:44:21 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>>> Does the accompanying testcase fails in Synth/Nimbus L&F, it seems not? >> >> No. I double-checked: >> A. If I explicitly set the L&F to Nimbus: this test does fail in this PR, >> and it does not fail in master. >> B. The SynthPasswordFieldUI descends from BasicTextUI, so it should not be >> impacted by this PR. >> >> ... and in reviewing the SynthPasswordFieldUI I found the same code (copied >> & pasted) from BasicPasswordFieldUI. I removed that, because with this PR it >> should not be necessary. (And I did NOT find other instances of the same >> code copied and pasted anywhere else.) >> >>> Also, it will be useful to extend the test case to test all installed L&F >>> since it it changing Basic L&F code? >> >> I'm not sure what you're asking here. >> >> I tweaked the test so it aborts if it is NOT interacting with a BasicTextUI. >> (Off the top of my head I don't know of any JPasswordField UI's that don't >> subclass BasicTextUI, but they might exist somewhere.) > >> > Does the accompanying testcase fails in Synth/Nimbus L&F, it seems not? >> >> No. I double-checked: A. If I explicitly set the L&F to Nimbus: this test >> does fail in this PR, and it does not fail in master. B. The >> SynthPasswordFieldUI descends from BasicTextUI, so it should not be impacted >> by this PR. > > Since you mentioned at the beginning that` AquaTextPasswordFieldUI (and > SynthPasswordFieldUI) do NOT extend the BasicPasswordFieldUI, so they weren't > inheriting this solution`. > so I assumed the regression testcase should fail in Aqua and Nimbus wiithout > your fix but it fails in Aqua and not in Nimbus so I asked. > It seems you have modified your initial PR description to NOT include Synth > now so it means the problem exists only in Aqua L&F in JDK mainline, am i > correct? > > >> >> Also, it will be useful to extend the test case to test all installed >> L&F since it it changing Basic L&F code? >> >> I'm not sure what you're asking here. > > I meant the present regression testcase only tests the system L&F of the > platform it is run ie Metal in WIndows, Linux and Aqua in Mac and does not > test all L&Fs like Nimbus, Motif, WIndows, GTK so you should check for > `UIManager.getInstalledLookAndFeels() `and iterate and test all the installed > L&Fs of the current platform..You will find many such example of the L&F > testing in `test/javax/swing` folder (@prsadhuk on a related note: your comments about SetEchoCharWordOpsTest led me to more research which resulted in writing up bug report 9078598 .) ------------- PR Comment: https://git.openjdk.org/jdk/pull/25443#issuecomment-2948367723
