On Fri, 6 Jun 2025 07:51:11 GMT, Jeremy Wood <d...@openjdk.org> wrote:
>> Make sure AquaTextPasswordFieldUI can't use selectWordAction. >> >> The core problem here was we could call selectWordAction in the Aqua LAF on >> a JPasswordField. This problem was already solved in the >> BasicPasswordFieldUI and SynthPasswordFieldUI, but the >> AquaTextPasswordFieldUI does NOT extend the BasicPasswordFieldUI, so it >> wasn't inheriting this solution. >> >> So the problem is partially about multiple inheritance. >> >> My first response at solving the problem is just to move the existing >> solution to the parent BasicTextUI class and use an instanceof to make sure >> it is only applied to JPasswordFields. >> >> There may be many different philosophies/recommendations on how to resolve >> this; I'm open to suggestions. > > Jeremy Wood has updated the pull request incrementally with two additional > commits since the last revision: > > - 8354646: change Error to RuntimeException > > This is in response to: > https://github.com/openjdk/jdk/pull/25443#discussion_r2131681790 > - 8354646: removing catching RuntimeException > > This is in response to: > https://github.com/openjdk/jdk/pull/25443#discussion_r2131680454 src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 654: > 652: // Create the action map for Password Field. This map > provides > 653: // same actions for double mouse click and > 654: // and for triple mouse click (see bugs 4231444, 8354646). Replace “create” with ”edit”? src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java line 662: > 660: map.put(DefaultEditorKit.selectWordAction, a); > 661: } > 662: } I think we should still remove `selectWordAction` even if `selectLineAction` doesn't exist. test/jdk/javax/swing/plaf/basic/BasicPasswordFieldUI/PasswordSelectionWordTest.java line 76: > 74: "because the JPasswordField UI was " + field.getUI()); > 75: return; > 76: } I'd say that this is unexpected, and it's better to throw an exception and fail the test. If not fail the test, then throw `jtreg.SkippedException` to indicate the test doesn't run. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25443#discussion_r2131918285 PR Review Comment: https://git.openjdk.org/jdk/pull/25443#discussion_r2131922909 PR Review Comment: https://git.openjdk.org/jdk/pull/25443#discussion_r2131930548
