On Tue, 18 Aug 2026 15:01:27 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: Revert comment modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 59: > 57: import test.com.sun.javafx.scene.control.infrastructure.StageLoader; > 58: > 59: unnecessary blank line, please remove. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2131: > 2129: } > 2130: > 2131: label.setContentDisplay(ContentDisplay.GRAPHIC_ONLY); here we tested mnemonic behavior when content display was set to `GRAPHIC_ONLY`. would it make sense to set the content display to `TEXT_ONLY` and re-check as a part of this test? modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2143: > 2141: assertFalse(containsMnemonicNode.get()); > 2142: > 2143: sl.dispose(); suggestion: to make sure the test cleans the environment even when it fails, you could create a `private StageLoader stageLoader` field instead of a local variable, and dispose of it if it's not `null` in `@AfterEach`. alternatively, you'll need to dispose of it in a `finally` block. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2159: > 2157: tk.firePulse(); > 2158: > 2159: label.setText("foo_bar"); I am getting SOOB exception if I do `label.setText("test_(t)");` here modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java line 2172: > 2170: // mnemonic -> empty > 2171: label.setText(""); > 2172: tk.firePulse(); could we also check the mnemonic registration? the registration should exist before L2171 and be removed after L2172 ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3806602226 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3806532214 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3806545760 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3806933324 PR Review Comment: https://git.openjdk.org/jfx/pull/2253#discussion_r3806613277
