On Thu, 20 Aug 2026 18:27:40 GMT, eduardsdv <[email protected]> wrote:
>> This PR fixes the StringIndexOutOfBoundsException, that occurs when an empty >> text is set to a Labeled, that previously contained a mnemonic. >> >> The reason for the error is that the ``updateDisplayedText(double, >> double)``, which also updates ``containsMnemonic`` flag, was not invoked if >> the text was empty. The value of this flag was still ``true`` but the index >> of the mnemonic character inside ``MnemonicInfo`` had already been updated >> to -1. This led to the StringIndexOutOfBoundsException in the line 611. >> >> I fixed it by moving the call to ``updateDisplayedText(double, double)`` >> outside the if-clause, so that it is always called when the text is being >> laid out. This should not affect performance because the method already >> checks whether recalculation is required. If not, it exits quickly. >> >> --------- >> - [X] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > eduardsdv has updated the pull request incrementally with one additional > commit since the last revision: > > 8389970: Disable mnemonic processing on mac modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2160: > 2158: label.setContentDisplay(ContentDisplay.GRAPHIC_ONLY); > 2159: stageLoader = new StageLoader(label); > 2160: Toolkit.getToolkit().firePulse(); // No NPE Suggestion: assertDoesNotThrow(() -> Toolkit.getToolkit().firePulse()); modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2217: > 2215: System.out.println(sb.toString()); > 2216: throw e; > 2217: } Usually a `catch` is not needed, JUnit will show the error and the stacktrace. This is usually rather an anti-pattern. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2260: > 2258: } > 2259: > 2260: private void checkMnemonics() { Minor, how about we name this `verifyMnemonics` to make clear we do JUnit verifications here. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2264: > 2262: if (stageLoader == null) { > 2263: stageLoader = new StageLoader(label); > 2264: } I would recommend to do that once in the test as a first step and remove it from here. Will make the separation of concerns much easier. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3851674501 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3851662484 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3851640199 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3851648116
