Mnemonic Parsing doesn't appear to work properly

[John Hendrikx]

Looking a bit further, and I noticed this code has recently been 
rewritten. The old code used StringBuffer and wasn't that easy to read, 
but the new code although easier to read seems to contain several 
changes in how things used to work.

The most noticable is that it doesn't stop after it finds the first 
mnemonic (probably because it is now building a new string and stopping 
half way would result in an incomplete string -- the old code instead 
would delete parts from a copy so it could stop early, although that has 
some bugs too I noticed).

The new code therefore finds the last mnemonic instead of the first, but 
unfortunately, there is an off by one error that becomes gradually worse 
as more underscores are encountered.

Compare the outputs (Input: How_to_test_mnemonics):

Old version 
(https://github.com/openjdk/jfx/blob/jfx16/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextBinding.java):
=========================

Resulting Label = Howto_test_mnemonics; mnemonicIndex = 3; mnemonic = t

Current version (master):
=========================

Resulting Label = Howtotestmnemonics; mnemonicIndex = 11; mnemonic = m

It looks like a significant change in how things used to work which the 
tests didn't catch at all.

--John

On 29/10/2021 21:45, John Hendrikx wrote:
> Hm, I can confirm it appears under the "e", also under Windows, but I
> suspect the platform will make no difference.
>
> I tested a bit further, and I found the following:
>
> 1) The mnemonic key seems to be "m", and it appears under the "e"
> because it deleted two more underscores (index is off by 2). What makes
> this hard to discover however is my second finding:
>
> 2) Checkbox doesn't toggle at all when you use the correct mnemonic, a
> Button however works.
>
> 3) If you add an "onAction" to the Checkbox that prints something on the
> console, you can see the mnemonic working, but the Checkbox is still not
> toggled.
>
> I checked the mnemonic parsing code, and if you run it in isolation, it
> finds the first underscore and has the correct index
> (TextBinding#parseAndSplit). I stopped there, but perhaps the code
> actually runs for each underscore found (it updates the text, perhaps
> this triggers another parsing run).
>
> --John
>
> On 29/10/2021 19:53, Scott Palmer wrote:
>> I looked in the bug database and while there are lots of bugs reported
>> against the mnemonic parsing, they claim to be fixed.
>>
>> After noticing something peculiar on a control in my app (it was
>> showing a
>> file path and I hadn't disabled the mnemonic parsing) I did some tests.
>>
>> The documentation is unclear on what should happen if multiple
>> underscores
>> are in the text.  It just says if the mnemonic parsing character '_'
>> appears that the key combination will be based on the succeeding
>> character.  I presume that it should pick only ONE of the underscores and
>> subsequent character, either the first or the last would make sense.
>> However, the behavior is that an unrelated character is shown as the
>> mnemonic and even it doesn't actually do anything.  Tested with JDK
>> 17.0.1
>> and JavaFX 17.0.1.
>>
>> Does this behave as expected for you?
>>
>>
>> package mnemonic_parsing_bug;
>>
>> import javafx.application.Application;
>> import javafx.geometry.Insets;
>> import javafx.scene.Scene;
>> import javafx.scene.control.CheckBox;
>> import javafx.scene.control.Label;
>> import javafx.scene.layout.VBox;
>> import javafx.stage.Stage;
>>
>> public class Main extends Application {
>>
>>     public static void main(String[] args) {
>>         launch(args);
>>     }
>>
>>     @Override
>>     public void start(Stage primaryStage) throws Exception {
>>         Label titleLabel = new Label("""
>>             The CheckBox below has text set to 'How_to_Test_Mnemonics'.
>>             On Windows press the Alt key to see where the underlined
>> character appears.
>>             It shows under the 'e' in Mnemonics for me, and I can't
>> figure
>> out
>>             what key combination (if any) actually does something.
>>             [JavaFX Version %s]""".formatted(System.getProperty(
>> "javafx.version")) );
>>         titleLabel.setMnemonicParsing(false);
>>         CheckBox mnemonicCB = new CheckBox("How_to_Test_Mnemonics");
>>         mnemonicCB.setMnemonicParsing(true);
>>         VBox box = new VBox(8,titleLabel, mnemonicCB);
>>         box.setPadding(new Insets(16));
>>         Scene scene = new Scene(box);
>>         primaryStage.setTitle("Mnemonic Parsing Bug");
>>         primaryStage.setScene(scene);
>>         primaryStage.show();
>>     }
>>
>> }
>>
>